[PATCH] D114833: [modules] Fix ambiguous name lookup for enum constants from hidden submodules.

[Volodymyr Sapsai via Phabricator via cfe-commits]

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 at redefinition-c-tagtypes.m:39 {{type 'enum NSMyEnum' has incompatible definitions}}
+  // expected-error at redefinition-c-tagtypes.m:43 {{type 'enum NSMyEnum' has incompatible definitions}}
   // expected-note at 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;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114833.390868.patch
Type: text/x-patch
Size: 1980 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211201/17caa0e6/attachment.bin>