vsapsai created this revision.
vsapsai added reviewers: bruno, jansvoboda11, rsmith.
Herald added a subscriber: ributzka.
vsapsai requested review of this revision.
Herald added a project: clang.
Fix errors like
clang/test/Modules/redefinition-c-tagtypes.m:36:10: error: reference to 'FST'
is ambiguous
return FST;
^
clang/test/Modules/redefinition-c-tagtypes.m:24:3: note: candidate found by
name lookup is 'FST'
FST = 22,
^
clang/test/Modules/Inputs/F.framework/PrivateHeaders/NS.h:7:3: note:
candidate found by name lookup is 'FST'
FST = 22,
^
The name lookup is ambiguous because we have 2 `EnumConstantDecl` for the
same identifier - one from hidden submodule, another non-modular from .m
file. One way to avoid ambiguity is for decls to have the same canonical
decl. But in this case the hidden decl is deserialized during lexing,
right before non-modular decl is created. So `ASTDeclReader` cannot do
anything useful in `mergeMergeable(Mergeable<EnumConstantDecl>*)` and
cannot make different decls have the same canonical decl.
The fix is roughly a follow-up to D31778 <https://reviews.llvm.org/D31778>. As
a duplicate non-modular
`EnumDecl` is dropped, we are dropping its constants too and remove them
from the global name lookup. Another option was to avoid adding
`EnumConstantDecl` to `IdResolver` when we know they are parsed for
comparison only. But we still need that global name lookup for constants
referencing other constants from the same enum, like `MinXOther = MinX`.
So instead we remove the constants after `ParseEnumBody` is done.
rdar://82908206
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D114833
Files:
clang/lib/Sema/SemaDecl.cpp
clang/test/Modules/redefinition-c-tagtypes.m
Index: clang/test/Modules/redefinition-c-tagtypes.m
===================================================================
--- clang/test/Modules/redefinition-c-tagtypes.m
+++ clang/test/Modules/redefinition-c-tagtypes.m
@@ -32,6 +32,10 @@
TRD = 55
};
+int testReferencingNSEConstant() {
+ return FST;
+}
+
#define NS_ENUM(_type, _name) \
enum _name : _type _name; \
enum _name : _type
@@ -42,7 +46,11 @@
MinXOther = MinX,
#else
MinXOther = TRD, // expected-note {{enumerator 'MinXOther' with value 55
here}}
- // expected-error@redefinition-c-tagtypes.m:39 {{type 'enum NSMyEnum' has
incompatible definitions}}
+ // expected-error@redefinition-c-tagtypes.m:43 {{type 'enum NSMyEnum' has
incompatible definitions}}
// expected-note@Inputs/F.framework/PrivateHeaders/NS.h:18 {{enumerator
'MinXOther' with value 11 here}}
#endif
};
+
+int testReferencingNSMyEnumConstant() {
+ return MinX;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16598,6 +16598,19 @@
// Make the previous decl visible.
makeMergedDefinitionVisible(SkipBody.Previous);
+ // For the ignored `EnumDecl` definition remove its `EnumConstantDecl` from
+ // global name lookup to avoid future ambiguous lookups. Doing this for
ObjC/C
+ // to mimic early return in `Sema::ActOnTag` that avoids calling
+ // `PushOnScopeChains` for duplicate `TagDecl` definitions. Doing it only for
+ // `EnumDecl` among all `TagDecl` because enum constants have global name
+ // lookup while record field lookup is limited to a record.
+ if (!getLangOpts().CPlusPlus) {
+ if (const auto *ED = dyn_cast<EnumDecl>(SkipBody.New))
+ for (EnumDecl::enumerator_iterator EC = ED->enumerator_begin(),
+ End = ED->enumerator_end();
+ EC != End; ++EC)
+ IdResolver.RemoveDecl(*EC);
+ }
return true;
}
Index: clang/test/Modules/redefinition-c-tagtypes.m
===================================================================
--- clang/test/Modules/redefinition-c-tagtypes.m
+++ clang/test/Modules/redefinition-c-tagtypes.m
@@ -32,6 +32,10 @@
TRD = 55
};
+int testReferencingNSEConstant() {
+ return FST;
+}
+
#define NS_ENUM(_type, _name) \
enum _name : _type _name; \
enum _name : _type
@@ -42,7 +46,11 @@
MinXOther = MinX,
#else
MinXOther = TRD, // expected-note {{enumerator 'MinXOther' with value 55 here}}
- // expected-error@redefinition-c-tagtypes.m:39 {{type 'enum NSMyEnum' has incompatible definitions}}
+ // expected-error@redefinition-c-tagtypes.m:43 {{type 'enum NSMyEnum' has incompatible definitions}}
// expected-note@Inputs/F.framework/PrivateHeaders/NS.h:18 {{enumerator 'MinXOther' with value 11 here}}
#endif
};
+
+int testReferencingNSMyEnumConstant() {
+ return MinX;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16598,6 +16598,19 @@
// Make the previous decl visible.
makeMergedDefinitionVisible(SkipBody.Previous);
+ // For the ignored `EnumDecl` definition remove its `EnumConstantDecl` from
+ // global name lookup to avoid future ambiguous lookups. Doing this for ObjC/C
+ // to mimic early return in `Sema::ActOnTag` that avoids calling
+ // `PushOnScopeChains` for duplicate `TagDecl` definitions. Doing it only for
+ // `EnumDecl` among all `TagDecl` because enum constants have global name
+ // lookup while record field lookup is limited to a record.
+ if (!getLangOpts().CPlusPlus) {
+ if (const auto *ED = dyn_cast<EnumDecl>(SkipBody.New))
+ for (EnumDecl::enumerator_iterator EC = ED->enumerator_begin(),
+ End = ED->enumerator_end();
+ EC != End; ++EC)
+ IdResolver.RemoveDecl(*EC);
+ }
return true;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
