r240204 - [modules] When determining whether a definition of a class is visible, check all modules even if we've already found a definition that's not visible.

[Richard Smith]

Author: rsmith
Date: Fri Jun 19 20:05:19 2015
New Revision: 240204

URL: http://llvm.org/viewvc/llvm-project?rev=240204&view=rev
Log:
[modules] When determining whether a definition of a class is visible, check all modules even if we've already found a definition that's not visible.

Added:
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/a.h
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/c.h
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h
    cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap
    cfe/trunk/test/Modules/merge-class-definition-visibility.cpp
Modified:
    cfe/trunk/lib/Sema/SemaType.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=240204&r1=240203&r2=240204&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Fri Jun 19 20:05:19 2015
@@ -5973,10 +5973,18 @@ bool Sema::hasVisibleDefinition(NamedDec
   }
   assert(D && "missing definition for pattern of instantiated definition");
 
-  // FIXME: If we merged any other decl into D, and that declaration is visible,
-  // then we should consider a definition to be visible.
   *Suggested = D;
-  return LookupResult::isVisible(*this, D);
+  if (LookupResult::isVisible(*this, 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 false;
 }
 
 /// Locks in the inheritance model for the given class and all of its bases.

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=240204&r1=240203&r2=240204&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jun 19 20:05:19 2015
@@ -3890,6 +3890,9 @@ void ASTDeclReader::UpdateDecl(Decl *D,
 
     case UPD_DECL_EXPORTED:
       unsigned SubmoduleID = readSubmoduleID(Record, Idx);
+      auto *Exported = cast<NamedDecl>(D);
+      if (auto *TD = dyn_cast<TagDecl>(Exported))
+        Exported = TD->getDefinition();
       Module *Owner = SubmoduleID ? Reader.getSubmodule(SubmoduleID) : nullptr;
       if (Reader.getContext().getLangOpts().ModulesLocalVisibility) {
         // FIXME: This doesn't send the right notifications if there are

Added: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/a.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/a.h?rev=240204&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/a.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/a.h Fri Jun 19 20:05:19 2015
@@ -0,0 +1 @@
+struct A {};

Added: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h?rev=240204&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/b.h Fri Jun 19 20:05:19 2015
@@ -0,0 +1,2 @@
+// Include definition of A into the same module as c.h
+#include "a.h"

Added: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/c.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/c.h?rev=240204&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/c.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/c.h Fri Jun 19 20:05:19 2015
@@ -0,0 +1 @@
+struct A;

Added: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h?rev=240204&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h (added)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/d.h Fri Jun 19 20:05:19 2015
@@ -0,0 +1 @@
+#include "a.h"

Added: cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap?rev=240204&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap (added)
+++ cfe/trunk/test/Modules/Inputs/merge-class-definition-visibility/modmap Fri Jun 19 20:05:19 2015
@@ -0,0 +1,7 @@
+module Def1 {
+  module B { header "b.h" }
+  module C { header "c.h" }
+}
+module Def2 {
+  header "d.h"
+}

Added: cfe/trunk/test/Modules/merge-class-definition-visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/merge-class-definition-visibility.cpp?rev=240204&view=auto
==============================================================================
--- cfe/trunk/test/Modules/merge-class-definition-visibility.cpp (added)
+++ cfe/trunk/test/Modules/merge-class-definition-visibility.cpp Fri Jun 19 20:05:19 2015
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%S/Inputs/merge-class-definition-visibility/modmap \
+// RUN:            -I%S/Inputs/merge-class-definition-visibility \
+// RUN:            -fmodules-cache-path=%t %s -verify
+// expected-no-diagnostics
+
+#include "c.h"
+template<typename T> struct X { T t; };
+typedef X<A> XA;
+
+#include "d.h"
+// Ensure that this triggers the import of the second definition from d.h,
+// which is necessary to make the definition of A visible in the template
+// instantiation.
+XA xa;