r233341 - [modules] Handle defining a class template on top of an existing imported-but-not-visible definition.

Richard Smith richard-llvm at metafoo.co.uk
Thu Mar 26 17:41:58 PDT 2015


Author: rsmith
Date: Thu Mar 26 19:41:57 2015
New Revision: 233341

URL: http://llvm.org/viewvc/llvm-project?rev=233341&view=rev
Log:
[modules] Handle defining a class template on top of an existing imported-but-not-visible definition.

Modified:
    cfe/trunk/include/clang/Sema/Lookup.h
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaLookup.cpp
    cfe/trunk/lib/Sema/SemaTemplate.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/Lookup.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=233341&r1=233340&r2=233341&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Lookup.h (original)
+++ cfe/trunk/include/clang/Sema/Lookup.h Thu Mar 26 19:41:57 2015
@@ -291,9 +291,6 @@ public:
     if (!D->isHidden())
       return true;
 
-    if (SemaRef.ActiveTemplateInstantiations.empty())
-      return false;
-
     // During template instantiation, we can refer to hidden declarations, if
     // they were visible in any module along the path of instantiation.
     return isVisibleSlow(SemaRef, D);

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=233341&r1=233340&r2=233341&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 26 19:41:57 2015
@@ -5344,7 +5344,8 @@ public:
                                 SourceLocation ModulePrivateLoc,
                                 SourceLocation FriendLoc,
                                 unsigned NumOuterTemplateParamLists,
-                            TemplateParameterList **OuterTemplateParamLists);
+                            TemplateParameterList **OuterTemplateParamLists,
+                                bool *SkipBody = nullptr);
 
   void translateTemplateArguments(const ASTTemplateArgsPtr &In,
                                   TemplateArgumentListInfo &Out);

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=233341&r1=233340&r2=233341&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Mar 26 19:41:57 2015
@@ -11297,7 +11297,8 @@ Decl *Sema::ActOnTag(Scope *S, unsigned
                                                ModulePrivateLoc,
                                                /*FriendLoc*/SourceLocation(),
                                                TemplateParameterLists.size()-1,
-                                               TemplateParameterLists.data());
+                                               TemplateParameterLists.data(),
+                                               SkipBody);
         return Result.get();
       } else {
         // The "template<>" header is extraneous.
@@ -11696,8 +11697,9 @@ Decl *Sema::ActOnTag(Scope *S, unsigned
                     TSK_ExplicitSpecialization;
               }
 
+              NamedDecl *Hidden = nullptr;
               if (SkipBody && getLangOpts().CPlusPlus &&
-                  !hasVisibleDefinition(Def, &Def)) {
+                  !hasVisibleDefinition(Def, &Hidden)) {
                 // There is a definition of this tag, but it is not visible. We
                 // explicitly make use of C++'s one definition rule here, and
                 // assume that this definition is identical to the hidden one
@@ -11705,8 +11707,8 @@ Decl *Sema::ActOnTag(Scope *S, unsigned
                 // use it in place of this one.
                 *SkipBody = true;
                 if (auto *Listener = getASTMutationListener())
-                  Listener->RedefinedHiddenDefinition(Def, KWLoc);
-                Def->setHidden(false);
+                  Listener->RedefinedHiddenDefinition(Hidden, KWLoc);
+                Hidden->setHidden(false);
                 return Def;
               } else if (!IsExplicitSpecializationAfterInstantiation) {
                 // A redeclaration in function prototype scope in C isn't

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=233341&r1=233340&r2=233341&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu Mar 26 19:41:57 2015
@@ -1217,11 +1217,27 @@ llvm::DenseSet<Module*> &Sema::getLookup
 /// path (by instantiating a template, you allow it to see the declarations that
 /// your module can see, including those later on in your module).
 bool LookupResult::isVisibleSlow(Sema &SemaRef, NamedDecl *D) {
-  assert(D->isHidden() && !SemaRef.ActiveTemplateInstantiations.empty() &&
-         "should not call this: not in slow case");
+  assert(D->isHidden() && "should not call this: not in slow case");
   Module *DeclModule = D->getOwningModule();
   assert(DeclModule && "hidden decl not from a module");
 
+  // If this declaration is not at namespace scope nor module-private,
+  // then it is visible if its lexical parent has a visible definition.
+  DeclContext *DC = D->getLexicalDeclContext();
+  if (!D->isModulePrivate() &&
+      DC && !DC->isFileContext() && !isa<LinkageSpecDecl>(DC)) {
+    NamedDecl *Hidden;
+    if (SemaRef.hasVisibleDefinition(cast<NamedDecl>(DC), &Hidden)) {
+      if (SemaRef.ActiveTemplateInstantiations.empty()) {
+        // Cache the fact that this declaration is implicitly visible because
+        // its parent has a visible definition.
+        D->setHidden(false);
+      }
+      return true;
+    }
+    return false;
+  }
+
   // Find the extra places where we need to look.
   llvm::DenseSet<Module*> &LookupModules = SemaRef.getLookupModules();
   if (LookupModules.empty())

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=233341&r1=233340&r2=233341&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Thu Mar 26 19:41:57 2015
@@ -12,6 +12,7 @@
 #include "TreeTransform.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
@@ -836,7 +837,8 @@ Sema::CheckClassTemplate(Scope *S, unsig
                          AccessSpecifier AS, SourceLocation ModulePrivateLoc,
                          SourceLocation FriendLoc,
                          unsigned NumOuterTemplateParamLists,
-                         TemplateParameterList** OuterTemplateParamLists) {
+                         TemplateParameterList** OuterTemplateParamLists,
+                         bool *SkipBody) {
   assert(TemplateParams && TemplateParams->size() > 0 &&
          "No template parameters");
   assert(TUK != TUK_Reference && "Can only declare or define class templates");
@@ -993,6 +995,23 @@ Sema::CheckClassTemplate(Scope *S, unsig
     // Check for redefinition of this class template.
     if (TUK == TUK_Definition) {
       if (TagDecl *Def = PrevRecordDecl->getDefinition()) {
+        // If we have a prior definition that is not visible, treat this as
+        // simply making that previous definition visible.
+        NamedDecl *Hidden = nullptr;
+        if (SkipBody && !hasVisibleDefinition(Def, &Hidden)) {
+          *SkipBody = true;
+          auto *Tmpl = cast<CXXRecordDecl>(Hidden)->getDescribedClassTemplate();
+          assert(Tmpl && "original definition of a class template is not a "
+                         "class template?");
+          if (auto *Listener = getASTMutationListener()) {
+            Listener->RedefinedHiddenDefinition(Hidden, KWLoc);
+            Listener->RedefinedHiddenDefinition(Tmpl, KWLoc);
+          }
+          Hidden->setHidden(false);
+          Tmpl->setHidden(false);
+          return Def;
+        }
+
         Diag(NameLoc, diag::err_redefinition) << Name;
         Diag(Def->getLocation(), diag::note_previous_definition);
         // FIXME: Would it make sense to try to "forget" the previous

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=233341&r1=233340&r2=233341&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h (original)
+++ cfe/trunk/test/Modules/Inputs/submodules-merge-defs/defs.h Thu Mar 26 19:41:57 2015
@@ -1,6 +1,16 @@
-struct A {};
+struct A { int a_member; };
+namespace { inline int use_a(A a) { return a.a_member; } }
+
 class B {
   struct Inner1 {};
+public:
   struct Inner2;
 };
+// Check that lookup and access checks are performed in the right context.
 struct B::Inner2 : Inner1 {};
+
+// Check that base-specifiers are correctly disambiguated.
+template<int N> struct C_Base { struct D { constexpr operator int() const { return 0; } }; };
+const int C_Const = 0;
+struct C1 : C_Base<C_Base<0>::D{}> {} extern c1;
+struct C2 : C_Base<C_Const<0>::D{} extern c2;

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=233341&r1=233340&r2=233341&view=diff
==============================================================================
--- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original)
+++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Thu Mar 26 19:41:57 2015
@@ -1,5 +1,5 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -x c++ -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery
 
 // Trigger import of definitions, but don't make them visible.
 #include "empty.h"
@@ -7,7 +7,22 @@
 A pre_a; // expected-error {{must be imported}} expected-error {{must use 'struct'}}
 // expected-note at defs.h:1 {{here}}
 
+B::Inner2 pre_bi; // expected-error +{{must be imported}}
+// expected-note at defs.h:4 +{{here}}
+// expected-note at defs.h:10 +{{here}}
+
+C_Base<1> pre_cb1; // expected-error +{{must be imported}}
+// expected-note at defs.h:13 +{{here}}
+C1 pre_c1; // expected-error +{{must be imported}} expected-error {{must use 'struct'}}
+// expected-note at defs.h:15 +{{here}}
+C2 pre_c2; // expected-error +{{must be imported}} expected-error {{must use 'struct'}}
+// expected-note at defs.h:16 +{{here}}
+
 // Make definitions from second module visible.
 #include "import-and-redefine.h"
 
 A post_a;
+B::Inner2 post_bi;
+C_Base<1> post_cb1;
+C1 c1;
+C2 c2;





More information about the cfe-commits mailing list