Hide upb_MiniTableField.submsg_index with new `UPB_PRIVATE()` macro

The fields of upb_MiniTableField are intended to be internal-only, accessed only through public functions like `upb_MiniTable_GetSubMessageTable()`.  But over time, clients have started accessing many of these fields directly.  This is an easy mistake to make, as there is no clear signal that the fields should not be used in applications.  This makes the implementation difficult to change without breaking users.

The new `UPB_PRIVATE()` macro appends an unpredictable string to each private symbol.  This makes it very difficult to accidentally use a private symbol, since users would need to write something like `field->submsg_index_dont_copy_me__upb_internal_use_only`.  This is still possible to do, but it leaves a clear wart in the code showing that an an encapsulation break has occurred.  The `UPB_PRIVATE()` macro itself is defined in `port/def.inc`, which users cannot include directly.

Once we land this, more such CLs will follow for the other fields of `upb_MiniTable*`.  We will add inline functions as needed to provide the semantic functionality needed by users.

PiperOrigin-RevId: 523166901

BUILD

@@ -329,6 +329,7 @@ cc_test(
         ":collections",
         ":message_accessors",
         ":mini_table_internal",
+        ":port",
         ":upb",
         "//upb/test:test_messages_proto2_upb_proto",
         "//upb/test:test_messages_proto3_upb_proto",

upb/message/accessors.c

@@ -199,7 +199,8 @@ upb_UnknownToMessageRet upb_MiniTable_PromoteUnknownToMessage(
   upb_Message* message = NULL;
   // Callers should check that message is not set first before calling
   // PromotoUnknownToMessage.
-  UPB_ASSERT(mini_table->subs[field->submsg_index].submsg == sub_mini_table);
+  UPB_ASSERT(upb_MiniTable_GetSubMessageTable(mini_table, field) ==
+             sub_mini_table);
   bool is_oneof = _upb_MiniTableField_InOneOf(field);
   if (!is_oneof || _upb_getoneofcase_field(msg, field) == field->number) {
     UPB_ASSERT(upb_Message_GetMessage(msg, field, NULL) == NULL);
@@ -286,7 +287,7 @@ upb_MapInsertStatus upb_Message_InsertMapEntry(upb_Map* map,
                                                upb_Message* map_entry_message,
                                                upb_Arena* arena) {
   const upb_MiniTable* map_entry_mini_table =
-      mini_table->subs[field->submsg_index].submsg;
+      mini_table->subs[field->UPB_PRIVATE(submsg_index)].submsg;
   UPB_ASSERT(map_entry_mini_table);
   UPB_ASSERT(map_entry_mini_table->field_count == 2);
   const upb_MiniTableField* map_entry_key_field =
@@ -311,7 +312,7 @@ upb_UnknownToMessage_Status upb_MiniTable_PromoteUnknownToMap(
     upb_Message* msg, const upb_MiniTable* mini_table,
     const upb_MiniTableField* field, int decode_options, upb_Arena* arena) {
   const upb_MiniTable* map_entry_mini_table =
-      mini_table->subs[field->submsg_index].submsg;
+      mini_table->subs[field->UPB_PRIVATE(submsg_index)].submsg;
   UPB_ASSERT(map_entry_mini_table);
   UPB_ASSERT(map_entry_mini_table);
   UPB_ASSERT(map_entry_mini_table->field_count == 2);

upb/message/accessors.h

@@ -552,7 +552,7 @@ UPB_API_INLINE void upb_Message_SetMessage(upb_Message* msg,
   UPB_ASSUME(!upb_IsRepeatedOrMap(field));
   UPB_ASSUME(_upb_MiniTableField_GetRep(field) ==
              UPB_SIZE(kUpb_FieldRep_4Byte, kUpb_FieldRep_8Byte));
-  UPB_ASSERT(mini_table->subs[field->submsg_index].submsg);
+  UPB_ASSERT(mini_table->subs[field->UPB_PRIVATE(submsg_index)].submsg);
   _upb_Message_SetNonExtensionField(msg, field, &sub_message);
 }
 
@@ -564,7 +564,7 @@ UPB_API_INLINE upb_Message* upb_Message_GetOrCreateMutableMessage(
   upb_Message* sub_message = *UPB_PTR_AT(msg, field->offset, upb_Message*);
   if (!sub_message) {
     const upb_MiniTable* sub_mini_table =
-        mini_table->subs[field->submsg_index].submsg;
+        mini_table->subs[field->UPB_PRIVATE(submsg_index)].submsg;
     UPB_ASSERT(sub_mini_table);
     sub_message = _upb_Message_New(sub_mini_table, arena);
     *UPB_PTR_AT(msg, field->offset, upb_Message*) = sub_message;

upb/message/accessors_test.cc

@@ -49,6 +49,9 @@
 #include "upb/wire/common.h"
 #include "upb/wire/decode.h"
 
+// Must be last
+#include "upb/port/def.inc"
+
 namespace {
 
 // Proto2 test messages field numbers used for reflective access.
@@ -580,10 +583,10 @@ upb_MiniTable* CreateMiniTableWithEmptySubTables(upb_Arena* arena) {
   // Initialize sub table to null. Not using upb_MiniTable_SetSubMessage
   // since it checks ->ext on parameter.
   upb_MiniTableSub* sub = const_cast<upb_MiniTableSub*>(
-      &table->subs[table->fields[1].submsg_index]);
+      &table->subs[table->fields[1].UPB_PRIVATE(submsg_index)]);
   sub->submsg = nullptr;
   sub = const_cast<upb_MiniTableSub*>(
-      &table->subs[table->fields[2].submsg_index]);
+      &table->subs[table->fields[2].UPB_PRIVATE(submsg_index)]);
   sub->submsg = nullptr;
   return table;
 }
@@ -605,10 +608,10 @@ upb_MiniTable* CreateMiniTableWithEmptySubTablesForMaps(upb_Arena* arena) {
   // Initialize sub table to null. Not using upb_MiniTable_SetSubMessage
   // since it checks ->ext on parameter.
   upb_MiniTableSub* sub = const_cast<upb_MiniTableSub*>(
-      &table->subs[table->fields[1].submsg_index]);
+      &table->subs[table->fields[1].UPB_PRIVATE(submsg_index)]);
   sub->submsg = nullptr;
   sub = const_cast<upb_MiniTableSub*>(
-      &table->subs[table->fields[2].submsg_index]);
+      &table->subs[table->fields[2].UPB_PRIVATE(submsg_index)]);
   sub->submsg = nullptr;
   return table;
 }

upb/message/copy.c

@@ -101,7 +101,7 @@ upb_Map* upb_Map_DeepClone(const upb_Map* map, upb_CType key_type,
   while (upb_Map_Next(map, &key, &val, &iter)) {
     const upb_MiniTableField* value_field = &map_entry_table->fields[1];
     const upb_MiniTable* value_sub =
-        (value_field->submsg_index != kUpb_NoSub)
+        (value_field->UPB_PRIVATE(submsg_index) != kUpb_NoSub)
             ? upb_MiniTable_GetSubMessageTable(map_entry_table, value_field)
             : NULL;
     upb_CType value_field_type = upb_MiniTableField_CType(value_field);
@@ -122,7 +122,7 @@ static upb_Map* upb_Message_Map_DeepClone(const upb_Map* map,
                                           upb_Message* clone,
                                           upb_Arena* arena) {
   const upb_MiniTable* map_entry_table =
-      mini_table->subs[field->submsg_index].submsg;
+      mini_table->subs[field->UPB_PRIVATE(submsg_index)].submsg;
   UPB_ASSERT(map_entry_table);
 
   const upb_MiniTableField* key_field = &map_entry_table->fields[0];
@@ -166,7 +166,7 @@ static bool upb_Message_Array_DeepClone(const upb_Array* array,
   _upb_MiniTableField_CheckIsArray(field);
   upb_Array* cloned_array = upb_Array_DeepClone(
       array, upb_MiniTableField_CType(field),
-      field->submsg_index != kUpb_NoSub
+      field->UPB_PRIVATE(submsg_index) != kUpb_NoSub
           ? upb_MiniTable_GetSubMessageTable(mini_table, field)
           : NULL,
       arena);

upb/mini_table/common.h

@@ -112,14 +112,27 @@ UPB_API_INLINE bool upb_MiniTableField_HasPresence(
   }
 }
 
+// Returns the MiniTable for this message field.  If the field is unlinked,
+// returns NULL.
 UPB_API_INLINE const upb_MiniTable* upb_MiniTable_GetSubMessageTable(
     const upb_MiniTable* mini_table, const upb_MiniTableField* field) {
-  return mini_table->subs[field->submsg_index].submsg;
+  UPB_ASSERT(upb_MiniTableField_CType(field) == kUpb_CType_Message);
+  return mini_table->subs[field->UPB_PRIVATE(submsg_index)].submsg;
 }
 
+// Returns the MiniTableEnum for this enum field.  If the field is unlinked,
+// returns NULL.
 UPB_API_INLINE const upb_MiniTableEnum* upb_MiniTable_GetSubEnumTable(
     const upb_MiniTable* mini_table, const upb_MiniTableField* field) {
-  return mini_table->subs[field->submsg_index].subenum;
+  UPB_ASSERT(upb_MiniTableField_CType(field) == kUpb_CType_Enum);
+  return mini_table->subs[field->UPB_PRIVATE(submsg_index)].subenum;
+}
+
+// Returns true if this MiniTable field is linked to a MiniTable for the
+// sub-message.
+UPB_API_INLINE bool upb_MiniTable_MessageFieldIsLinked(
+    const upb_MiniTable* mini_table, const upb_MiniTableField* field) {
+  return upb_MiniTable_GetSubMessageTable(mini_table, field) != NULL;
 }
 
 // If this field is in a oneof, returns the first field in the oneof.

upb/mini_table/decode.c

@@ -172,9 +172,9 @@ static void upb_MiniTable_SetTypeAndSub(upb_MiniTableField* field,
   }
 
   if (upb_MiniTable_HasSub(field, msg_modifiers)) {
-    field->submsg_index = sub_count ? (*sub_count)++ : 0;
+    field->UPB_PRIVATE(submsg_index) = sub_count ? (*sub_count)++ : 0;
   } else {
-    field->submsg_index = kUpb_NoSub;
+    field->UPB_PRIVATE(submsg_index) = kUpb_NoSub;
   }
 
   if (upb_MtDecoder_FieldIsPackable(field) &&
@@ -1046,7 +1046,8 @@ bool upb_MiniTable_SetSubMessage(upb_MiniTable* table,
       return false;
   }
 
-  upb_MiniTableSub* table_sub = (void*)&table->subs[field->submsg_index];
+  upb_MiniTableSub* table_sub =
+      (void*)&table->subs[field->UPB_PRIVATE(submsg_index)];
   table_sub->submsg = sub;
   return true;
 }
@@ -1058,7 +1059,8 @@ bool upb_MiniTable_SetSubEnum(upb_MiniTable* table, upb_MiniTableField* field,
                  (uintptr_t)(table->fields + table->field_count));
   UPB_ASSERT(sub);
 
-  upb_MiniTableSub* table_sub = (void*)&table->subs[field->submsg_index];
+  upb_MiniTableSub* table_sub =
+      (void*)&table->subs[field->UPB_PRIVATE(submsg_index)];
   table_sub->subenum = sub;
   return true;
 }

upb/mini_table/field_internal.h

@@ -40,7 +40,11 @@ struct upb_MiniTableField {
   uint32_t number;
   uint16_t offset;
   int16_t presence;       // If >0, hasbit_index.  If <0, ~oneof_index
-  uint16_t submsg_index;  // kUpb_NoSub if descriptortype != MESSAGE/GROUP/ENUM
+
+  // Indexes into `upb_MiniTable.subs`
+  // Will be set to `kUpb_NoSub` if `descriptortype` != MESSAGE/GROUP/ENUM
+  uint16_t UPB_PRIVATE(submsg_index);
+
   uint8_t descriptortype;
 
   // upb_FieldMode | upb_LabelFlags | (upb_FieldRep << kUpb_FieldRep_Shift)

upb/port/def.inc

@@ -200,6 +200,8 @@
 /* UPB_PTRADD(ptr, ofs): add pointer while avoiding "NULL + 0" UB */
 #define UPB_PTRADD(ptr, ofs) ((ofs) ? (ptr) + (ofs) : (ptr))
 
+#define UPB_PRIVATE(x) x##_dont_copy_me__upb_internal_use_only
+
 /* Configure whether fasttable is switched on or not. *************************/
 
 #ifdef __has_attribute

upb/port/undef.inc

@@ -70,3 +70,4 @@
 #undef UPB_IS_GOOGLE3
 #undef UPB_ATOMIC
 #undef UPB_USE_C11_ATOMICS
+#undef UPB_PRIVATE

upb/wire/decode.c

@@ -220,7 +220,7 @@ static void _upb_Decoder_Munge(int type, wireval* val) {
 static upb_Message* _upb_Decoder_NewSubMessage(
     upb_Decoder* d, const upb_MiniTableSub* subs,
     const upb_MiniTableField* field) {
-  const upb_MiniTable* subl = subs[field->submsg_index].submsg;
+  const upb_MiniTable* subl = subs[field->UPB_PRIVATE(submsg_index)].submsg;
   UPB_ASSERT(subl);
   upb_Message* msg = _upb_Message_New(subl, &d->arena);
   if (!msg) _upb_Decoder_ErrorJmp(d, kUpb_DecodeStatus_OutOfMemory);
@@ -259,7 +259,7 @@ static const char* _upb_Decoder_DecodeSubMessage(
     upb_Decoder* d, const char* ptr, upb_Message* submsg,
     const upb_MiniTableSub* subs, const upb_MiniTableField* field, int size) {
   int saved_delta = upb_EpsCopyInputStream_PushLimit(&d->input, ptr, size);
-  const upb_MiniTable* subl = subs[field->submsg_index].submsg;
+  const upb_MiniTable* subl = subs[field->UPB_PRIVATE(submsg_index)].submsg;
   UPB_ASSERT(subl);
   ptr = _upb_Decoder_RecurseSubMessage(d, ptr, submsg, subl, DECODE_NOGROUP);
   upb_EpsCopyInputStream_PopLimit(&d->input, ptr, saved_delta);
@@ -290,7 +290,7 @@ UPB_FORCEINLINE
 static const char* _upb_Decoder_DecodeKnownGroup(
     upb_Decoder* d, const char* ptr, upb_Message* submsg,
     const upb_MiniTableSub* subs, const upb_MiniTableField* field) {
-  const upb_MiniTable* subl = subs[field->submsg_index].submsg;
+  const upb_MiniTable* subl = subs[field->UPB_PRIVATE(submsg_index)].submsg;
   UPB_ASSERT(subl);
   return _upb_Decoder_DecodeGroup(d, ptr, submsg, subl, field->number);
 }
@@ -354,7 +354,7 @@ static const char* _upb_Decoder_DecodeEnumArray(upb_Decoder* d, const char* ptr,
                                                 const upb_MiniTableSub* subs,
                                                 const upb_MiniTableField* field,
                                                 wireval* val) {
-  const upb_MiniTableEnum* e = subs[field->submsg_index].subenum;
+  const upb_MiniTableEnum* e = subs[field->UPB_PRIVATE(submsg_index)].subenum;
   if (!_upb_Decoder_CheckEnum(d, ptr, msg, e, field, val)) return ptr;
   void* mem = UPB_PTR_AT(_upb_array_ptr(arr), arr->size * 4, void);
   arr->size++;
@@ -425,7 +425,7 @@ static const char* _upb_Decoder_DecodeEnumPacked(
     upb_Decoder* d, const char* ptr, upb_Message* msg, upb_Array* arr,
     const upb_MiniTableSub* subs, const upb_MiniTableField* field,
     wireval* val) {
-  const upb_MiniTableEnum* e = subs[field->submsg_index].subenum;
+  const upb_MiniTableEnum* e = subs[field->UPB_PRIVATE(submsg_index)].subenum;
   int saved_limit = upb_EpsCopyInputStream_PushLimit(&d->input, ptr, val->size);
   char* out = UPB_PTR_AT(_upb_array_ptr(arr), arr->size * 4, void);
   while (!_upb_Decoder_IsDone(d, &ptr)) {
@@ -586,7 +586,7 @@ static const char* _upb_Decoder_DecodeToMap(upb_Decoder* d, const char* ptr,
   upb_Map* map = *map_p;
   upb_MapEntry ent;
   UPB_ASSERT(upb_MiniTableField_Type(field) == kUpb_FieldType_Message);
-  const upb_MiniTable* entry = subs[field->submsg_index].submsg;
+  const upb_MiniTable* entry = subs[field->UPB_PRIVATE(submsg_index)].submsg;
 
   UPB_ASSERT(entry->field_count == 2);
   UPB_ASSERT(!upb_IsRepeatedOrMap(&entry->fields[0]));
@@ -647,7 +647,8 @@ static const char* _upb_Decoder_DecodeToSubMessage(
   int type = field->descriptortype;
 
   if (UPB_UNLIKELY(op == kUpb_DecodeOp_Enum) &&
-      !_upb_Decoder_CheckEnum(d, ptr, msg, subs[field->submsg_index].subenum,
+      !_upb_Decoder_CheckEnum(d, ptr, msg,
+                              subs[field->UPB_PRIVATE(submsg_index)].subenum,
                               field, val)) {
     return ptr;
   }
@@ -963,7 +964,7 @@ static void _upb_Decoder_CheckUnlinked(const upb_MiniTable* mt,
                                        int* op) {
   // If sub-message is not linked, treat as unknown.
   if (field->mode & kUpb_LabelFlags_IsExtension) return;
-  const upb_MiniTableSub* sub = &mt->subs[field->submsg_index];
+  const upb_MiniTableSub* sub = &mt->subs[field->UPB_PRIVATE(submsg_index)];
   if (!sub->submsg) *op = kUpb_DecodeOp_UnknownField;
 }
 

upb/wire/encode.c

@@ -263,7 +263,7 @@ static void encode_scalar(upb_encstate* e, const void* _field_mem,
     case kUpb_FieldType_Group: {
       size_t size;
       void* submsg = *(void**)field_mem;
-      const upb_MiniTable* subm = subs[f->submsg_index].submsg;
+      const upb_MiniTable* subm = subs[f->UPB_PRIVATE(submsg_index)].submsg;
       if (submsg == NULL) {
         return;
       }
@@ -277,7 +277,7 @@ static void encode_scalar(upb_encstate* e, const void* _field_mem,
     case kUpb_FieldType_Message: {
       size_t size;
       void* submsg = *(void**)field_mem;
-      const upb_MiniTable* subm = subs[f->submsg_index].submsg;
+      const upb_MiniTable* subm = subs[f->UPB_PRIVATE(submsg_index)].submsg;
       if (submsg == NULL) {
         return;
       }
@@ -366,7 +366,7 @@ static void encode_array(upb_encstate* e, const upb_Message* msg,
     case kUpb_FieldType_Group: {
       const void* const* start = _upb_array_constptr(arr);
       const void* const* ptr = start + arr->size;
-      const upb_MiniTable* subm = subs[f->submsg_index].submsg;
+      const upb_MiniTable* subm = subs[f->UPB_PRIVATE(submsg_index)].submsg;
       if (--e->depth == 0) encode_err(e, kUpb_EncodeStatus_MaxDepthExceeded);
       do {
         size_t size;
@@ -381,7 +381,7 @@ static void encode_array(upb_encstate* e, const upb_Message* msg,
     case kUpb_FieldType_Message: {
       const void* const* start = _upb_array_constptr(arr);
       const void* const* ptr = start + arr->size;
-      const upb_MiniTable* subm = subs[f->submsg_index].submsg;
+      const upb_MiniTable* subm = subs[f->UPB_PRIVATE(submsg_index)].submsg;
       if (--e->depth == 0) encode_err(e, kUpb_EncodeStatus_MaxDepthExceeded);
       do {
         size_t size;
@@ -420,7 +420,7 @@ static void encode_map(upb_encstate* e, const upb_Message* msg,
                        const upb_MiniTableSub* subs,
                        const upb_MiniTableField* f) {
   const upb_Map* map = *UPB_PTR_AT(msg, f->offset, const upb_Map*);
-  const upb_MiniTable* layout = subs[f->submsg_index].submsg;
+  const upb_MiniTable* layout = subs[f->UPB_PRIVATE(submsg_index)].submsg;
   UPB_ASSERT(layout->field_count == 2);
 
   if (map == NULL) return;

upbc/protoc-gen-upb.cc

@@ -1115,7 +1115,7 @@ bool TryFillTableEntry(const DefPoolPair& pools, upb::FieldDefPtr field,
   }
 
   if (field.ctype() == kUpb_CType_Message) {
-    uint64_t idx = mt_f->submsg_index;
+    uint64_t idx = mt_f->UPB_PRIVATE(submsg_index);
     if (idx > 255) return false;
     data |= idx << 16;
 
@@ -1263,9 +1263,9 @@ std::string FieldInitializer(upb::FieldDefPtr field,
         "{$0, $1, $2, $3, $4, $5}", field64->number,
         ArchDependentSize(field32->offset, field64->offset),
         ArchDependentSize(field32->presence, field64->presence),
-        field64->submsg_index == kUpb_NoSub
+        field64->UPB_PRIVATE(submsg_index) == kUpb_NoSub
             ? "kUpb_NoSub"
-            : absl::StrCat(field64->submsg_index).c_str(),
+            : absl::StrCat(field64->UPB_PRIVATE(submsg_index)).c_str(),
         field64->descriptortype, GetModeInit(field32, field64));
   }
 }
@@ -1311,7 +1311,7 @@ void WriteMessage(upb::MessageDefPtr message, const DefPoolPair& pools,
 
   for (int i = 0; i < mt_64->field_count; i++) {
     const upb_MiniTableField* f = &mt_64->fields[i];
-    if (f->submsg_index != kUpb_NoSub) {
+    if (f->UPB_PRIVATE(submsg_index) != kUpb_NoSub) {
       subs.push_back(GetSub(message.FindFieldByNumber(f->number)));
     }
   }