[modules] Fix merging support for forward-declared enums with fixed underlying types. A visible declaration is enough to make the type complete, but not enough to make the definition visible.

llvm-svn: 241743

4 files changed, 24 insertions, 8 deletions

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 0590f436288..b5a1b0be7c4 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1387,7 +1387,8 @@ public:
 
   /// Determine if \p D has a visible definition. If not, suggest a declaration
   /// that should be made visible to expose the definition.
-  bool hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested);
+  bool hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested,
+                            bool OnlyNeedComplete = false);
   bool hasVisibleDefinition(const NamedDecl *D) {
     NamedDecl *Hidden;
     return hasVisibleDefinition(const_cast<NamedDecl*>(D), &Hidden);
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 1347efcc74c..02a31ef8d79 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6369,7 +6369,12 @@ bool Sema::RequireCompleteType(SourceLocation Loc, QualType T,
 /// \param D The definition of the entity.
 /// \param Suggested Filled in with the declaration that should be made visible
 ///        in order to provide a definition of this entity.
-bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested) {
+/// \param OnlyNeedComplete If \c true, we only need the type to be complete,
+///        not defined. This only matters for enums with a fixed underlying
+///        type, since in all other cases, a type is complete if and only if it
+///        is defined.
+bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested,
+                                bool OnlyNeedComplete) {
   // Easy case: if we don't have modules, all declarations are visible.
   if (!getLangOpts().Modules && !getLangOpts().ModulesLocalVisibility)
     return true;
@@ -6387,11 +6392,13 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested) {
   } else if (auto *ED = dyn_cast<EnumDecl>(D)) {
     while (auto *NewED = ED->getInstantiatedFromMemberEnum())
       ED = NewED;
-    if (ED->isFixed()) {
-      // If the enum has a fixed underlying type, any declaration of it will do.
+    if (OnlyNeedComplete && ED->isFixed()) {
+      // If the enum has a fixed underlying type, and we're only looking for a
+      // complete type (not a definition), any visible declaration of it will
+      // do.
       *Suggested = nullptr;
       for (auto *Redecl : ED->redecls()) {
-        if (LookupResult::isVisible(*this, Redecl))
+        if (isVisible(Redecl))
           return true;
         if (Redecl->isThisDeclarationADefinition() ||
             (Redecl->isCanonicalDecl() && !*Suggested))
@@ -6404,14 +6411,14 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl **Suggested) {
   assert(D && "missing definition for pattern of instantiated definition");
 
   *Suggested = D;
-  if (LookupResult::isVisible(*this, D))
+  if (isVisible(D))
     return true;
 
   // The external source may have additional definitions of this type that are
   // visible, so complete the redeclaration chain now and ask again.
   if (auto *Source = Context.getExternalSource()) {
     Source->CompleteRedeclChain(D);
-    return LookupResult::isVisible(*this, D);
+    return isVisible(D);
   }
 
   return false;
@@ -6465,7 +6472,7 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
     // If we know about the definition but it is not visible, complain.
     NamedDecl *SuggestedDef = nullptr;
     if (!Diagnoser.Suppressed && Def &&
-        !hasVisibleDefinition(Def, &SuggestedDef))
+        !hasVisibleDefinition(Def, &SuggestedDef, /*OnlyNeedComplete*/true))
       diagnoseMissingImport(Loc, SuggestedDef, /*NeedDefinition*/true);
 
     // We lock in the inheritance model once somebody has asked us to ensure
diff --git a/clang/test/Modules/Inputs/submodules-merge-defs/defs.h b/clang/test/Modules/Inputs/submodules-merge-defs/defs.h
index bda0567c93a..07dfac7ee65 100644
--- a/clang/test/Modules/Inputs/submodules-merge-defs/defs.h
+++ b/clang/test/Modules/Inputs/submodules-merge-defs/defs.h
@@ -94,3 +94,6 @@ namespace MergeFunctionTemplateSpecializations {
   };
   using xiq = X<int>::Q<int>;
 }
+
+enum ScopedEnum : int;
+enum ScopedEnum : int { a, b, c };
diff --git a/clang/test/Modules/submodules-merge-defs.cpp b/clang/test/Modules/submodules-merge-defs.cpp
index 92c784440b9..016b8a8f47a 100644
--- a/clang/test/Modules/submodules-merge-defs.cpp
+++ b/clang/test/Modules/submodules-merge-defs.cpp
@@ -69,6 +69,11 @@ J<> pre_j; // expected-error {{declaration of 'J' must be imported}}
 #endif
 // expected-note@defs.h:51 +{{here}}
 
+ScopedEnum pre_scopedenum; // expected-error {{must be imported}} expected-error {{must use 'enum'}}
+// expected-note@defs.h:99 {{here}}
+enum ScopedEnum : int;
+ScopedEnum pre_scopedenum_declared; // ok
+
 // Make definitions from second module visible.
 #ifdef TEXTUAL
 #include "import-and-redefine.h"