r241743 - [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.

[Richard Smith]

Author: rsmith
Date: Wed Jul  8 16:49:31 2015
New Revision: 241743

URL: http://llvm.org/viewvc/llvm-project?rev=241743&view=rev
Log:
[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.

Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaType.cpp
    cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h
    cfe/trunk/test/Modules/submodules-merge-defs.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=241743&r1=241742&r2=241743&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Jul  8 16:49:31 2015
@@ -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);

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=241743&r1=241742&r2=241743&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Wed Jul  8 16:49:31 2015
@@ -6369,7 +6369,12 @@ bool Sema::RequireCompleteType(SourceLoc
 /// \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(NamedDec
   } 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(NamedDec
   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(Sourc
     // 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

Modified: cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h?rev=241743&r1=241742&r2=241743&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h (original)
+++ cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h Wed Jul  8 16:49:31 2015
@@ -94,3 +94,6 @@ namespace MergeFunctionTemplateSpecializ
   };
   using xiq = X<int>::Q<int>;
 }
+
+enum ScopedEnum : int;
+enum ScopedEnum : int { a, b, c };

Modified: cfe/trunk/test/Modules/submodules-merge-defs.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules-merge-defs.cpp?rev=241743&r1=241742&r2=241743&view=diff
==============================================================================
--- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original)
+++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Wed Jul  8 16:49:31 2015
@@ -69,6 +69,11 @@ J<> pre_j; // expected-error {{declarati
 #endif
 // expected-note at defs.h:51 +{{here}}
 
+ScopedEnum pre_scopedenum; // expected-error {{must be imported}} expected-error {{must use 'enum'}}
+// expected-note at 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"