Skip to content

Commit 4c5c4b2

Browse files
habermancopybara-github
haberman
authored and
copybara-github
committed
Added more validations of syntax and edition when parsing descriptors.
Also removed `syntax` from upb's data structures and API surfaces. PiperOrigin-RevId: 850499478
1 parent 9eee325 commit 4c5c4b2

File tree

13 files changed

+159
-65
lines changed

13 files changed

+159
-65
lines changed

‎lua/def.c‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -462,12 +462,6 @@ static int lupb_MessageDef_IsMapEntry(lua_State* L) {
462462
return 1;
463463
}
464464

465-
static int lupb_MessageDef_Syntax(lua_State* L) {
466-
const upb_MessageDef* m = lupb_MessageDef_check(L, 1);
467-
lua_pushinteger(L, upb_MessageDef_Syntax(m));
468-
return 1;
469-
}
470-
471465
static int lupb_MessageDef_tostring(lua_State* L) {
472466
const upb_MessageDef* m = lupb_MessageDef_check(L, 1);
473467
lua_pushfstring(L, "<upb.MessageDef name=%s, field_count=%d>",
@@ -493,7 +487,6 @@ static const struct luaL_Reg lupb_MessageDef_m[] = {
493487
{"name", lupb_MessageDef_Name},
494488
{"oneof_count", lupb_MessageDef_OneofCount},
495489
{"oneofs", lupb_MessageDef_Oneofs},
496-
{"syntax", lupb_MessageDef_Syntax},
497490
{"_map_entry", lupb_MessageDef_IsMapEntry},
498491
{NULL, NULL}};
499492

@@ -656,12 +649,6 @@ static int lupb_FileDef_Pool(lua_State* L) {
656649
return 1;
657650
}
658651

659-
static int lupb_FileDef_Syntax(lua_State* L) {
660-
const upb_FileDef* f = lupb_FileDef_check(L, 1);
661-
lua_pushnumber(L, upb_FileDef_Syntax(f));
662-
return 1;
663-
}
664-
665652
static const struct luaL_Reg lupb_FileDef_m[] = {
666653
{"dep", lupb_FileDef_Dependency},
667654
{"depcount", lupb_FileDef_DependencyCount},
@@ -672,7 +659,6 @@ static const struct luaL_Reg lupb_FileDef_m[] = {
672659
{"name", lupb_FileDef_Name},
673660
{"package", lupb_FileDef_Package},
674661
{"defpool", lupb_FileDef_Pool},
675-
{"syntax", lupb_FileDef_Syntax},
676662
{NULL, NULL}};
677663

678664
/* lupb_DefPool
@@ -914,7 +900,4 @@ void lupb_def_registertypes(lua_State* L) {
914900
lupb_setfieldi(L, "DESCRIPTOR_TYPE_SFIXED64", kUpb_FieldType_SFixed64);
915901
lupb_setfieldi(L, "DESCRIPTOR_TYPE_SINT32", kUpb_FieldType_SInt32);
916902
lupb_setfieldi(L, "DESCRIPTOR_TYPE_SINT64", kUpb_FieldType_SInt64);
917-
918-
lupb_setfieldi(L, "SYNTAX_PROTO2", kUpb_Syntax_Proto2);
919-
lupb_setfieldi(L, "SYNTAX_PROTO3", kUpb_Syntax_Proto3);
920903
}

‎upb/reflection/common.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,6 @@
1414

1515
#include "upb/reflection/descriptor_bootstrap.h" // IWYU pragma: export
1616

17-
typedef enum {
18-
kUpb_Syntax_Proto2 = 2,
19-
kUpb_Syntax_Proto3 = 3,
20-
kUpb_Syntax_Editions = 99
21-
} upb_Syntax;
22-
2317
// Forward declarations for circular references.
2418
typedef struct upb_DefPool upb_DefPool;
2519
typedef struct upb_EnumDef upb_EnumDef;

‎upb/reflection/def.hpp‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,6 @@ class MessageDefPtr {
258258
return upb_MessageDef_ExtensionRangeCount(ptr_);
259259
}
260260

261-
upb_Syntax syntax() const { return upb_MessageDef_Syntax(ptr_); }
262-
263261
// These return null pointers if the field is not found.
264262
FieldDefPtr FindFieldByNumber(uint32_t number) const {
265263
return FieldDefPtr(upb_MessageDef_FindFieldByNumber(ptr_, number));
@@ -489,9 +487,6 @@ class FileDefPtr {
489487
// Package name for definitions inside the file (eg. "foo.bar").
490488
const char* package() const { return upb_FileDef_Package(ptr_); }
491489

492-
// Syntax for the file. Defaults to proto2.
493-
upb_Syntax syntax() const { return upb_FileDef_Syntax(ptr_); }
494-
495490
// Get the list of dependencies from the file. These are returned in the
496491
// order that they were added to the FileDefPtr.
497492
int dependency_count() const { return upb_FileDef_DependencyCount(ptr_); }

‎upb/reflection/field_def.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "upb/mini_table/sub.h"
3030
#include "upb/reflection/def.h"
3131
#include "upb/reflection/def_type.h"
32+
#include "upb/reflection/descriptor_bootstrap.h"
3233
#include "upb/reflection/internal/def_builder.h"
3334
#include "upb/reflection/internal/def_pool.h"
3435
#include "upb/reflection/internal/desc_state.h"
@@ -569,12 +570,12 @@ static void set_default_default(upb_DefBuilder* ctx, upb_FieldDef* f,
569570
static bool _upb_FieldDef_InferLegacyFeatures(
570571
upb_DefBuilder* ctx, upb_FieldDef* f,
571572
const UPB_DESC(FieldDescriptorProto*) proto,
572-
const UPB_DESC(FieldOptions*) options, upb_Syntax syntax,
573+
const UPB_DESC(FieldOptions*) options, google_protobuf_Edition edition,
573574
UPB_DESC(FeatureSet*) features) {
574575
bool ret = false;
575576

576577
if (UPB_DESC(FieldDescriptorProto_label)(proto) == kUpb_Label_Required) {
577-
if (syntax == kUpb_Syntax_Proto3) {
578+
if (edition == google_protobuf_EDITION_PROTO3) {
578579
_upb_DefBuilder_Errf(ctx, "proto3 fields cannot be required (%s)",
579580
f->full_name);
580581
}
@@ -618,15 +619,15 @@ static void _upb_FieldDef_Create(upb_DefBuilder* ctx, const char* prefix,
618619

619620
UPB_DEF_SET_OPTIONS(f->opts, FieldDescriptorProto, FieldOptions, field_proto);
620621

621-
upb_Syntax syntax = upb_FileDef_Syntax(f->file);
622+
UPB_DESC(Edition) edition = upb_FileDef_Edition(f->file);
622623
const UPB_DESC(FeatureSet*) unresolved_features =
623624
UPB_DESC(FieldOptions_features)(f->opts);
624625
bool implicit = false;
625626

626-
if (syntax != kUpb_Syntax_Editions) {
627+
if (_upb_DefBuilder_IsLegacyEdition(edition)) {
627628
upb_Message_Clear(UPB_UPCAST(ctx->legacy_features),
628629
UPB_DESC_MINITABLE(FeatureSet));
629-
if (_upb_FieldDef_InferLegacyFeatures(ctx, f, field_proto, f->opts, syntax,
630+
if (_upb_FieldDef_InferLegacyFeatures(ctx, f, field_proto, f->opts, edition,
630631
ctx->legacy_features)) {
631632
implicit = true;
632633
unresolved_features = ctx->legacy_features;
@@ -1009,7 +1010,7 @@ static void resolve_default(upb_DefBuilder* ctx, upb_FieldDef* f,
10091010
f->full_name);
10101011
}
10111012

1012-
if (upb_FileDef_Syntax(f->file) == kUpb_Syntax_Proto3) {
1013+
if (upb_FileDef_Edition(f->file) == google_protobuf_EDITION_PROTO3) {
10131014
_upb_DefBuilder_Errf(ctx,
10141015
"proto3 fields cannot have explicit defaults (%s)",
10151016
f->full_name);

‎upb/reflection/file_def.c‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ struct upb_FileDef {
5252
int top_lvl_ext_count;
5353
int service_count;
5454
int ext_count; // All exts in the file.
55-
upb_Syntax syntax;
5655
};
5756

5857
UPB_API const char* upb_FileDef_EditionName(int edition) {
@@ -94,8 +93,6 @@ UPB_DESC(Edition) upb_FileDef_Edition(const upb_FileDef* f) {
9493

9594
const char* _upb_FileDef_RawPackage(const upb_FileDef* f) { return f->package; }
9695

97-
upb_Syntax upb_FileDef_Syntax(const upb_FileDef* f) { return f->syntax; }
98-
9996
int upb_FileDef_TopLevelMessageCount(const upb_FileDef* f) {
10097
return f->top_lvl_msg_count;
10198
}
@@ -324,27 +321,30 @@ void _upb_FileDef_Create(upb_DefBuilder* ctx,
324321
file->package = NULL;
325322
}
326323

327-
// TODO: How should we validate this?
328-
file->edition = UPB_DESC(FileDescriptorProto_edition)(file_proto);
329-
330-
if (UPB_DESC(FileDescriptorProto_has_syntax)(file_proto)) {
331-
upb_StringView syntax = UPB_DESC(FileDescriptorProto_syntax)(file_proto);
324+
upb_StringView syntax = UPB_DESC(FileDescriptorProto_syntax)(file_proto);
332325

326+
if (UPB_DESC(FileDescriptorProto_has_edition)(file_proto)) {
327+
if (!streql_view(syntax, "editions")) {
328+
_upb_DefBuilder_Errf(ctx,
329+
"Setting edition requires that syntax=\"editions\", "
330+
"but syntax is \"" UPB_STRINGVIEW_FORMAT "\"",
331+
UPB_STRINGVIEW_ARGS(syntax));
332+
}
333+
file->edition = UPB_DESC(FileDescriptorProto_edition)(file_proto);
334+
} else if (UPB_DESC(FileDescriptorProto_has_syntax)(file_proto)) {
333335
if (streql_view(syntax, "proto2")) {
334-
file->syntax = kUpb_Syntax_Proto2;
335336
file->edition = UPB_DESC(EDITION_PROTO2);
336337
} else if (streql_view(syntax, "proto3")) {
337-
file->syntax = kUpb_Syntax_Proto3;
338338
file->edition = UPB_DESC(EDITION_PROTO3);
339339
} else if (streql_view(syntax, "editions")) {
340-
file->syntax = kUpb_Syntax_Editions;
341-
file->edition = UPB_DESC(FileDescriptorProto_edition)(file_proto);
340+
_upb_DefBuilder_Errf(
341+
ctx, "File has syntax=\"editions\", but no edition is specified");
342342
} else {
343343
_upb_DefBuilder_Errf(ctx, "Invalid syntax '" UPB_STRINGVIEW_FORMAT "'",
344344
UPB_STRINGVIEW_ARGS(syntax));
345345
}
346346
} else {
347-
file->syntax = kUpb_Syntax_Proto2;
347+
// The legacy default when no edition or syntax is specified is proto2.
348348
file->edition = UPB_DESC(EDITION_PROTO2);
349349
}
350350

‎upb/reflection/file_def.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ int upb_FileDef_PublicDependencyCount(const upb_FileDef* f);
3737
const upb_ServiceDef* upb_FileDef_Service(const upb_FileDef* f, int i);
3838
int upb_FileDef_ServiceCount(const upb_FileDef* f);
3939

40-
UPB_API upb_Syntax upb_FileDef_Syntax(const upb_FileDef* f);
41-
4240
const upb_EnumDef* upb_FileDef_TopLevelEnum(const upb_FileDef* f, int i);
4341
int upb_FileDef_TopLevelEnumCount(const upb_FileDef* f);
4442

‎upb/reflection/internal/def_builder.c‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,26 @@
77

88
#include "upb/reflection/internal/def_builder.h"
99

10+
#include <assert.h>
11+
#include <stdarg.h>
12+
#include <stdint.h>
1013
#include <string.h>
1114

1215
#include "upb/base/internal/log2.h"
16+
#include "upb/base/status.h"
17+
#include "upb/base/string_view.h"
1318
#include "upb/base/upcast.h"
19+
#include "upb/hash/common.h"
20+
#include "upb/hash/str_table.h"
1421
#include "upb/mem/alloc.h"
22+
#include "upb/mem/arena.h"
1523
#include "upb/message/copy.h"
16-
#include "upb/reflection/def_pool.h"
24+
#include "upb/reflection/def.h"
1725
#include "upb/reflection/def_type.h"
26+
#include "upb/reflection/descriptor_bootstrap.h"
1827
#include "upb/reflection/field_def.h"
1928
#include "upb/reflection/file_def.h"
29+
#include "upb/reflection/internal/def_pool.h"
2030
#include "upb/reflection/internal/strdup2.h"
2131
#include "upb/wire/decode.h"
2232

@@ -392,8 +402,8 @@ const UPB_DESC(FeatureSet*)
392402
assert(parent);
393403
if (!child) return parent;
394404

395-
if (child && !is_implicit &&
396-
upb_FileDef_Syntax(ctx->file) != kUpb_Syntax_Editions) {
405+
if (!is_implicit &&
406+
_upb_DefBuilder_IsLegacyEdition(upb_FileDef_Edition(ctx->file))) {
397407
_upb_DefBuilder_Errf(ctx, "Features can only be specified for editions");
398408
}
399409

‎upb/reflection/internal/def_builder.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,13 @@ UPB_INLINE void _upb_DefBuilder_CheckIdentFull(upb_DefBuilder* ctx,
158158
if (!good) _upb_DefBuilder_CheckIdentSlow(ctx, name, true);
159159
}
160160

161+
UPB_INLINE bool _upb_DefBuilder_IsLegacyEdition(google_protobuf_Edition edition) {
162+
// Should only be called for a real edition, not a placeholder like
163+
// EDITION_LEGACY.
164+
UPB_ASSERT(edition >= google_protobuf_EDITION_PROTO2);
165+
return edition <= google_protobuf_EDITION_PROTO3;
166+
}
167+
161168
// Returns true if the returned feature set is new and must be populated.
162169
bool _upb_DefBuilder_GetOrCreateFeatureSet(upb_DefBuilder* ctx,
163170
const UPB_DESC(FeatureSet*) parent,

‎upb/reflection/message_def.c‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,6 @@ const char* upb_MessageDef_Name(const upb_MessageDef* m) {
166166
return _upb_DefBuilder_FullToShort(m->full_name);
167167
}
168168

169-
upb_Syntax upb_MessageDef_Syntax(const upb_MessageDef* m) {
170-
return upb_FileDef_Syntax(m->file);
171-
}
172-
173169
const upb_FieldDef* upb_MessageDef_FindFieldByNumber(const upb_MessageDef* m,
174170
uint32_t i) {
175171
upb_value val;

‎upb/reflection/message_def.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,6 @@ const upb_MessageReservedRange* upb_MessageDef_ReservedRange(
146146
const upb_MessageDef* m, int i);
147147
int upb_MessageDef_ReservedRangeCount(const upb_MessageDef* m);
148148

149-
UPB_API upb_Syntax upb_MessageDef_Syntax(const upb_MessageDef* m);
150149
UPB_API upb_WellKnown upb_MessageDef_WellKnownType(const upb_MessageDef* m);
151150
UPB_API UPB_DESC(SymbolVisibility)
152151
upb_MessageDef_Visibility(const upb_MessageDef* m);

0 commit comments

Comments
 (0)