Skip to content

Commit fe64c41

Browse files
Mizuchimeta-codesync[bot]
authored andcommitted
Remove options when generating metadata
Summary: Previously we want to selectively generate annotations or nested types. We don't need this option anymore after the migration. Reviewed By: yfeldblum Differential Revision: D90144385 fbshipit-source-id: 282f054a94b9f0fa5a372f16f59de824669e6fbd
1 parent bf49ac7 commit fe64c41

File tree

5 files changed

+77
-137
lines changed

5 files changed

+77
-137
lines changed

third-party/thrift/src/thrift/lib/cpp2/gen/module_metadata_h.h

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,30 +37,25 @@ using ThriftService = ::apache::thrift::metadata::ThriftService;
3737
using ThriftServiceMetadataResponse =
3838
::apache::thrift::metadata::ThriftServiceMetadataResponse;
3939

40-
inline constexpr Options kGenerateAll = {
41-
.genAnnotations = true, .genNestedTypes = true};
42-
4340
template <typename T>
4441
class EnumMetadata {
4542
public:
46-
static void gen(ThriftMetadata& metadata) {
47-
genEnumMetadata<T>(metadata, kGenerateAll);
48-
}
43+
static void gen(ThriftMetadata& metadata) { genEnumMetadata<T>(metadata); }
4944
};
5045

5146
template <typename T>
5247
class StructMetadata {
5348
public:
5449
static const metadata::ThriftStruct& gen(ThriftMetadata& metadata) {
55-
return genStructMetadata<T>(metadata, kGenerateAll).metadata;
50+
return genStructMetadata<T>(metadata).metadata;
5651
}
5752
};
5853

5954
template <typename T>
6055
class ExceptionMetadata {
6156
public:
6257
static void gen(ThriftMetadata& metadata) {
63-
genExceptionMetadata<T>(metadata, kGenerateAll);
58+
genExceptionMetadata<T>(metadata);
6459
}
6560
};
6661

third-party/thrift/src/thrift/lib/cpp2/test/metadata/annotations_test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ TEST(Annotations, Exception) {
387387

388388
TEST(Annotations, Service) {
389389
metadata::ThriftMetadata md;
390-
auto service = genServiceMetadata<TestService>(md, {.genAnnotations = true});
390+
auto service = genServiceMetadata<TestService>(md);
391391
EXPECT_EQ(
392392
service.structured_annotations()->begin()->fields()["baz"].cv_string(),
393393
"0");

third-party/thrift/src/thrift/lib/cpp2/test/metadata/schema_comparison_test.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,7 @@ TYPED_TEST_SUITE(StructComparisonTest, Structureds);
6464
TYPED_TEST(StructComparisonTest, StructComparisonTest) {
6565
metadata::ThriftMetadata lhs, rhs;
6666
StructMetadata<TypeParam>::gen(lhs);
67-
genStructMetadata<TypeParam>(
68-
rhs, {.genAnnotations = true, .genNestedTypes = true});
67+
genStructMetadata<TypeParam>(rhs);
6968
EXPECT_EQ(lhs, rhs);
7069
EXPECT_GE(lhs.structs()->size(), 1); // sanity check
7170
}

third-party/thrift/src/thrift/lib/cpp2/util/SchemaToMetadata.cpp

Lines changed: 60 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -246,9 +246,7 @@ metadata::detail::LimitedVector<ThriftConstStruct> genStructuredAnnotations(
246246
} // namespace
247247

248248
GenMetadataResult<metadata::ThriftEnum> genEnumMetadata(
249-
metadata::ThriftMetadata& md,
250-
const syntax_graph::EnumNode& node,
251-
Options options) {
249+
metadata::ThriftMetadata& md, const syntax_graph::EnumNode& node) {
252250
auto name = getName(node);
253251
auto res = md.enums()->try_emplace(name);
254252
GenMetadataResult<metadata::ThriftEnum> ret{!res.second, res.first->second};
@@ -259,10 +257,8 @@ GenMetadataResult<metadata::ThriftEnum> genEnumMetadata(
259257
for (const auto& value : node.values()) {
260258
ret.metadata.elements()[value.i32()] = value.name();
261259
}
262-
if (options.genAnnotations) {
263-
ret.metadata.structured_annotations() =
264-
genStructuredAnnotations(node.definition().annotations());
265-
}
260+
ret.metadata.structured_annotations() =
261+
genStructuredAnnotations(node.definition().annotations());
266262
return ret;
267263
}
268264

@@ -298,31 +294,30 @@ metadata::ThriftPrimitiveType toThriftPrimitiveType(
298294
ThriftType genType(
299295
metadata::ThriftMetadata& md, const syntax_graph::TypeRef& type) {
300296
ThriftType ret;
301-
Options options = {.genAnnotations = true, .genNestedTypes = true};
302297
switch (type.kind()) {
303298
case syntax_graph::TypeRef::Kind::PRIMITIVE: {
304299
ret.t_primitive() = toThriftPrimitiveType(type.asPrimitive());
305300
return ret;
306301
}
307302
case syntax_graph::TypeRef::Kind::STRUCT: {
308-
auto res = genStructMetadata(md, type.asStructured(), options);
303+
auto res = genStructMetadata(md, type.asStructured());
309304
ret.t_struct().emplace().name() = *res.metadata.name();
310305
return ret;
311306
}
312307
case syntax_graph::TypeRef::Kind::UNION: {
313-
auto res = genStructMetadata(md, type.asStructured(), options);
308+
auto res = genStructMetadata(md, type.asStructured());
314309
ret.t_union().emplace().name() = *res.metadata.name();
315310
return ret;
316311
}
317312
case syntax_graph::TypeRef::Kind::EXCEPTION: {
318313
// Note: exception is considered t_struct in metadata's Type system.
319314
// There is no t_exception in metadata::ThriftType.
320-
auto res = genStructMetadata(md, type.asStructured(), options);
315+
auto res = genStructMetadata(md, type.asStructured());
321316
ret.t_struct().emplace().name() = *res.metadata.name();
322317
return ret;
323318
}
324319
case syntax_graph::TypeRef::Kind::ENUM: {
325-
auto res = genEnumMetadata(md, type.asEnum(), options);
320+
auto res = genEnumMetadata(md, type.asEnum());
326321
ret.t_enum().emplace().name() = *res.metadata.name();
327322
return ret;
328323
}
@@ -365,8 +360,7 @@ template <class Metadata>
365360
static auto genStructuredInMetadataMap(
366361
metadata::ThriftMetadata& md,
367362
std::map<std::string, Metadata>& metadataMap,
368-
const syntax_graph::StructuredNode& node,
369-
Options options) {
363+
const syntax_graph::StructuredNode& node) {
370364
auto name = getName(node);
371365
auto res = metadataMap.try_emplace(name);
372366
GenMetadataResult<Metadata> ret{!res.second, res.first->second};
@@ -384,39 +378,26 @@ static auto genStructuredInMetadataMap(
384378
f.name() = field.name();
385379
f.is_optional() =
386380
(field.presence() == syntax_graph::FieldPresenceQualifier::OPTIONAL_);
387-
if (options.genAnnotations) {
388-
f.structured_annotations() =
389-
genStructuredAnnotations(field.annotations());
390-
}
391-
if (options.genNestedTypes) {
392-
f.type() = genType(md, field.type());
393-
}
394-
}
395-
if (options.genAnnotations) {
396-
ret.metadata.structured_annotations() =
397-
genStructuredAnnotations(node.definition().annotations());
381+
f.structured_annotations() = genStructuredAnnotations(field.annotations());
382+
f.type() = genType(md, field.type());
398383
}
384+
ret.metadata.structured_annotations() =
385+
genStructuredAnnotations(node.definition().annotations());
399386
return ret;
400387
}
401388

402389
GenMetadataResult<metadata::ThriftStruct> genStructMetadata(
403-
metadata::ThriftMetadata& md,
404-
const syntax_graph::StructuredNode& node,
405-
Options options) {
406-
return genStructuredInMetadataMap(md, *md.structs(), node, options);
390+
metadata::ThriftMetadata& md, const syntax_graph::StructuredNode& node) {
391+
return genStructuredInMetadataMap(md, *md.structs(), node);
407392
}
408393

409394
GenMetadataResult<metadata::ThriftException> genExceptionMetadata(
410-
metadata::ThriftMetadata& md,
411-
const syntax_graph::ExceptionNode& node,
412-
Options options) {
413-
return genStructuredInMetadataMap(md, *md.exceptions(), node, options);
395+
metadata::ThriftMetadata& md, const syntax_graph::ExceptionNode& node) {
396+
return genStructuredInMetadataMap(md, *md.exceptions(), node);
414397
}
415398

416399
metadata::ThriftService genServiceMetadata(
417-
const syntax_graph::ServiceNode& node,
418-
metadata::ThriftMetadata& md,
419-
Options options) {
400+
const syntax_graph::ServiceNode& node, metadata::ThriftMetadata& md) {
420401
metadata::ThriftService ret;
421402
ret.name() = getName(node);
422403
ret.uri() = node.uri();
@@ -428,40 +409,33 @@ metadata::ThriftService genServiceMetadata(
428409
continue;
429410
}
430411
ret.functions()->emplace_back().name() = func.name();
431-
if (options.genNestedTypes) {
432-
if (auto stream = func.response().stream()) {
433-
ret.functions()->back().return_type()->t_stream().emplace();
434-
ret.functions()->back().return_type()->t_stream()->elemType() =
435-
std::make_unique<ThriftType>(genType(md, stream->payloadType()));
436-
if (auto retType = func.response().type()) {
437-
ret.functions()
438-
->back()
439-
.return_type()
440-
->t_stream()
441-
->initialResponseType() =
442-
std::make_unique<ThriftType>(genType(md, *retType));
443-
}
444-
} else if (auto sink = func.response().sink()) {
445-
ret.functions()->back().return_type()->t_sink().emplace();
446-
ret.functions()->back().return_type()->t_sink()->elemType() =
447-
std::make_unique<ThriftType>(genType(md, sink->payloadType()));
448-
ret.functions()->back().return_type()->t_sink()->finalResponseType() =
449-
std::make_unique<ThriftType>(
450-
genType(md, sink->finalResponseType()));
451-
if (auto retType = func.response().type()) {
452-
ret.functions()
453-
->back()
454-
.return_type()
455-
->t_sink()
456-
->initialResponseType() =
457-
std::make_unique<ThriftType>(genType(md, *retType));
458-
}
459-
} else if (auto retType = func.response().type()) {
460-
ret.functions()->back().return_type() = genType(md, *retType);
461-
} else {
462-
ret.functions()->back().return_type()->t_primitive() =
463-
metadata::ThriftPrimitiveType::THRIFT_VOID_TYPE;
412+
if (auto stream = func.response().stream()) {
413+
ret.functions()->back().return_type()->t_stream().emplace();
414+
ret.functions()->back().return_type()->t_stream()->elemType() =
415+
std::make_unique<ThriftType>(genType(md, stream->payloadType()));
416+
if (auto retType = func.response().type()) {
417+
ret.functions()
418+
->back()
419+
.return_type()
420+
->t_stream()
421+
->initialResponseType() =
422+
std::make_unique<ThriftType>(genType(md, *retType));
423+
}
424+
} else if (auto sink = func.response().sink()) {
425+
ret.functions()->back().return_type()->t_sink().emplace();
426+
ret.functions()->back().return_type()->t_sink()->elemType() =
427+
std::make_unique<ThriftType>(genType(md, sink->payloadType()));
428+
ret.functions()->back().return_type()->t_sink()->finalResponseType() =
429+
std::make_unique<ThriftType>(genType(md, sink->finalResponseType()));
430+
if (auto retType = func.response().type()) {
431+
ret.functions()->back().return_type()->t_sink()->initialResponseType() =
432+
std::make_unique<ThriftType>(genType(md, *retType));
464433
}
434+
} else if (auto retType = func.response().type()) {
435+
ret.functions()->back().return_type() = genType(md, *retType);
436+
} else {
437+
ret.functions()->back().return_type()->t_primitive() =
438+
metadata::ThriftPrimitiveType::THRIFT_VOID_TYPE;
465439
}
466440
ret.functions()->back().is_oneway() =
467441
func.qualifier() == type::FunctionQualifier::OneWay;
@@ -470,52 +444,39 @@ metadata::ThriftService genServiceMetadata(
470444
i.id() = static_cast<std::int16_t>(param.id());
471445
i.name() = param.name();
472446
i.is_optional() = false;
473-
if (options.genAnnotations) {
474-
i.structured_annotations() =
475-
genStructuredAnnotations(param.annotations());
476-
}
477-
if (options.genNestedTypes) {
478-
i.type() = genType(md, param.type());
479-
}
447+
i.structured_annotations() =
448+
genStructuredAnnotations(param.annotations());
449+
i.type() = genType(md, param.type());
480450
}
481451
for (const auto& exception : func.exceptions()) {
482452
auto& i = ret.functions()->back().exceptions()->emplace_back();
483453
i.id() = static_cast<std::int16_t>(exception.id());
484454
i.name() = exception.name();
485455
i.is_optional() = false;
486-
if (options.genAnnotations) {
487-
i.structured_annotations() =
488-
genStructuredAnnotations(exception.annotations());
456+
i.structured_annotations() =
457+
genStructuredAnnotations(exception.annotations());
458+
i.type() = genType(md, exception.type());
459+
if (exception.type().isStructured()) {
460+
// Mimicking the existing logic: we add all types in throw clause
461+
// into `exceptions` field as long as it's structured.
462+
// https://github.com/facebook/fbthrift/blob/v2025.11.03.00/thrift/compiler/generate/templates/cpp2/module_metadata.cpp.mustache#L153-L157
463+
genStructuredInMetadataMap(
464+
md, *md.exceptions(), exception.type().asStructured());
489465
}
490-
if (options.genNestedTypes) {
491-
i.type() = genType(md, exception.type());
492-
if (exception.type().isStructured()) {
493-
// Mimicking the existing logic: we add all types in throw clause
494-
// into `exceptions` field as long as it's structured.
495-
// https://github.com/facebook/fbthrift/blob/v2025.11.03.00/thrift/compiler/generate/templates/cpp2/module_metadata.cpp.mustache#L153-L157
496-
genStructuredInMetadataMap(
497-
md, *md.exceptions(), exception.type().asStructured(), options);
498-
}
499-
}
500-
}
501-
if (options.genAnnotations) {
502-
ret.functions()->back().structured_annotations() =
503-
genStructuredAnnotations(func.annotations());
504466
}
467+
ret.functions()->back().structured_annotations() =
468+
genStructuredAnnotations(func.annotations());
505469
}
506-
if (options.genAnnotations) {
507-
ret.structured_annotations() =
508-
genStructuredAnnotations(node.definition().annotations());
509-
}
470+
ret.structured_annotations() =
471+
genStructuredAnnotations(node.definition().annotations());
510472
return ret;
511473
}
512474

513475
const ThriftServiceContextRef* genServiceMetadataRecurse(
514476
const syntax_graph::ServiceNode& node,
515477
ThriftMetadata& metadata,
516478
std::vector<ThriftServiceContextRef>& services) {
517-
Options options = {.genAnnotations = true, .genNestedTypes = true};
518-
auto serviceMetadata = genServiceMetadata(node, metadata, options);
479+
auto serviceMetadata = genServiceMetadata(node, metadata);
519480
// We need to keep the index around because a reference or iterator could be
520481
// invalidated.
521482
auto selfIndex = services.size();

third-party/thrift/src/thrift/lib/cpp2/util/SchemaToMetadata.h

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,52 +47,37 @@ const auto& getDefinitionNodeWithLock() {
4747
return SchemaRegistry::get().getDefinitionNode<T>();
4848
}
4949

50-
struct Options {
51-
bool genAnnotations = false;
52-
bool genNestedTypes = false;
53-
};
54-
5550
// Generate metadata of `node` inside `md`, return the generated metadata.
5651
GenMetadataResult<metadata::ThriftEnum> genEnumMetadata(
57-
metadata::ThriftMetadata& md,
58-
const syntax_graph::EnumNode& node,
59-
Options options);
52+
metadata::ThriftMetadata& md, const syntax_graph::EnumNode& node);
6053

6154
template <class E>
62-
auto genEnumMetadata(metadata::ThriftMetadata& md, Options options) {
63-
return genEnumMetadata(md, getNodeWithLock<E>(), options);
55+
auto genEnumMetadata(metadata::ThriftMetadata& md) {
56+
return genEnumMetadata(md, getNodeWithLock<E>());
6457
}
6558

6659
GenMetadataResult<metadata::ThriftStruct> genStructMetadata(
67-
metadata::ThriftMetadata& md,
68-
const syntax_graph::StructuredNode& node,
69-
Options options);
60+
metadata::ThriftMetadata& md, const syntax_graph::StructuredNode& node);
7061

7162
template <class T>
72-
auto genStructMetadata(metadata::ThriftMetadata& md, Options options) {
73-
return genStructMetadata(md, getNodeWithLock<T>(), options);
63+
auto genStructMetadata(metadata::ThriftMetadata& md) {
64+
return genStructMetadata(md, getNodeWithLock<T>());
7465
}
7566

7667
GenMetadataResult<metadata::ThriftException> genExceptionMetadata(
77-
metadata::ThriftMetadata& md,
78-
const syntax_graph::ExceptionNode& node,
79-
Options options);
68+
metadata::ThriftMetadata& md, const syntax_graph::ExceptionNode& node);
8069

8170
template <class T>
82-
auto genExceptionMetadata(metadata::ThriftMetadata& md, Options options) {
83-
return genExceptionMetadata(md, getNodeWithLock<T>(), options);
71+
auto genExceptionMetadata(metadata::ThriftMetadata& md) {
72+
return genExceptionMetadata(md, getNodeWithLock<T>());
8473
}
8574

8675
metadata::ThriftService genServiceMetadata(
87-
const syntax_graph::ServiceNode& node,
88-
metadata::ThriftMetadata& md,
89-
Options options);
76+
const syntax_graph::ServiceNode& node, metadata::ThriftMetadata& md);
9077

9178
template <class Tag>
92-
metadata::ThriftService genServiceMetadata(
93-
metadata::ThriftMetadata& md, Options options) {
94-
return genServiceMetadata(
95-
getDefinitionNodeWithLock<Tag>().asService(), md, options);
79+
metadata::ThriftService genServiceMetadata(metadata::ThriftMetadata& md) {
80+
return genServiceMetadata(getDefinitionNodeWithLock<Tag>().asService(), md);
9681
}
9782

9883
const metadata::ThriftServiceContextRef* genServiceMetadataRecurse(

0 commit comments

Comments
 (0)