Skip to content

Commit

Permalink
use thrift.uri to get @cpp.Ref annotation
Browse files Browse the repository at this point in the history
Reviewed By: Alfus

Differential Revision: D30543147

fbshipit-source-id: a7b98f648353d007e46145d7be0b41aab4e557cb
  • Loading branch information
Mizuchi authored and facebook-github-bot committed Sep 5, 2021
1 parent 75ebac8 commit 3569010
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 19 deletions.
12 changes: 7 additions & 5 deletions thrift/compiler/ast/t_named.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,13 @@ void t_named::add_structured_annotation(std::unique_ptr<t_const> annot) {
}

const t_const* t_named::get_structured_annotation_or_null(
const char* path, const char* name) const {
for (const auto* annotation : structured_annotations_raw_) {
const t_type* type = annotation->type().get_type()->get_true_type();
if (type->program()->path() == path && type->get_name() == name) {
return annotation;
const char* uri) const {
for (const auto* annot : structured_annotations_raw_) {
if (const std::string* p =
annot->get_type()->get_annotation_or_null("thrift.uri")) {
if (*p == uri) {
return annot;
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions thrift/compiler/ast/t_named.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ class t_named : public t_node {
}
void add_structured_annotation(std::unique_ptr<t_const> annot);

const t_const* get_structured_annotation_or_null(
const char* path, const char* name) const;
const t_const* get_structured_annotation_or_null(const char* uri) const;

protected:
// t_named is abstract.
Expand All @@ -59,6 +58,8 @@ class t_named : public t_node {

private:
std::vector<std::shared_ptr<const t_const>> structured_annotations_;

// TODO(ytj): use thrift.uri --> t_const map for structured annotation
std::vector<const t_const*> structured_annotations_raw_;

// TODO(afuller): Remove everything below this comment. It is only provided
Expand Down
2 changes: 1 addition & 1 deletion thrift/compiler/gen/cpp/reference_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ reference_type find_ref_type(const t_field& node) {
}

if (const t_const* anno = node.get_structured_annotation_or_null(
"thrift/lib/thrift/annotation/cpp.thrift", "Ref")) {
"facebook.com/thrift/annotation/cpp/Ref")) {
for (const auto& kv : anno->value()->get_map()) {
if (kv.first->get_string() == "type") {
switch (static_cast<RefType>(kv.second->get_integer())) {
Expand Down
2 changes: 1 addition & 1 deletion thrift/compiler/lib/cpp2/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ inline bool field_has_isset(const t_field* field) {
inline bool is_lazy(const t_field* field) {
return field->has_annotation("cpp.experimental.lazy") ||
field->get_structured_annotation_or_null(
"thrift/lib/thrift/annotation/cpp.thrift", "Lazy") != nullptr;
"facebook.com/thrift/annotation/cpp/Lazy") != nullptr;
}

bool field_transitively_refers_to_unique(const t_field* field);
Expand Down
2 changes: 1 addition & 1 deletion thrift/compiler/sema/ast_validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ void validate_compatibility_with_lazy_field(

void validate_ref_annotation(diagnostic_context& ctx, const t_field& node) {
if (node.get_structured_annotation_or_null(
"thrift/lib/thrift/annotation/cpp.thrift", "Ref") &&
"facebook.com/thrift/annotation/cpp/Ref") &&
node.has_annotation(
{"cpp.ref", "cpp2.ref", "cpp.ref_type", "cpp2.ref_type"})) {
ctx.failure([&](auto& o) {
Expand Down
14 changes: 7 additions & 7 deletions thrift/compiler/test/compiler_failure_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ def test_structured_ref(self):
enum RefType {Unique, SharedConst, SharedMutable}
struct Ref {
1: RefType type;
}
} (thrift.uri = "facebook.com/thrift/annotation/cpp/Ref")
"""
),
)
Expand Down Expand Up @@ -753,12 +753,12 @@ def test_structured_ref(self):

self.assertEqual(ret, 1)
self.assertEqual(
err,
"[FAILURE:foo.thrift:10] The @cpp.Ref annotation cannot be combined "
"with the `cpp.ref` or `cpp.ref_type` annotations. Remove one of the"
" annotations from `field3`.\n"
"[FAILURE:foo.thrift:13] Structured annotation `Ref` is already"
" defined for `field4`.\n",
'\n' + err,
textwrap.dedent("""
[FAILURE:foo.thrift:10] The @cpp.Ref annotation cannot be combined with the `cpp.ref` or `cpp.ref_type` annotations. Remove one of the annotations from `field3`.
[FAILURE:foo.thrift:13] Structured annotation `Ref` is already defined for `field4`.
[FAILURE:foo.thrift:13] Structured annotation thrift.uri `facebook.com/thrift/annotation/cpp/Ref` is already defined for `field4`.
""")
)

def test_mixin_nonstruct_members(self):
Expand Down
4 changes: 2 additions & 2 deletions thrift/lib/thrift/annotation/cpp.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ enum RefType {
@scope.Field
struct Ref {
1: RefType type;
}
} (thrift.uri = "facebook.com/thrift/annotation/cpp/Ref")

@scope.Field
struct Lazy {
}
} (thrift.uri = "facebook.com/thrift/annotation/cpp/Lazy")

0 comments on commit 3569010

Please sign in to comment.