[modules] Track all default template arguments for a given parameter across

modules, and allow use of a default template argument if any of the parameters
providing it is visible.

llvm-svn: 239485

diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index 324f0e8..d4e090e 100644 (file)
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -217,18 +217,35 @@ public:
   }
 };
 
-/// Storage for a default argument.
+/// Storage for a default argument. This is conceptually either empty, or an
+/// argument value, or a pointer to a previous declaration that had a default
+/// argument.
+///
+/// However, this is complicated by modules: while we require all the default
+/// arguments for a template to be equivalent, there may be more than one, and
+/// we need to track all the originating parameters to determine if the default
+/// argument is visible.
 template<typename ParmDecl, typename ArgType>
 class DefaultArgStorage {
-  llvm::PointerUnion<ArgType, ParmDecl*> ValueOrInherited;
+  /// Storage for both the value *and* another parameter from which we inherit
+  /// the default argument. This is used when multiple default arguments for a
+  /// parameter are merged together from different modules.
+  struct Chain {
+    ParmDecl *PrevDeclWithDefaultArg;
+    ArgType Value;
+  };
+  static_assert(sizeof(Chain) == sizeof(void *) * 2,
+                "non-pointer argument type?");
+
+  llvm::PointerUnion3<ArgType, ParmDecl*, Chain*> ValueOrInherited;
 
   static ParmDecl *getParmOwningDefaultArg(ParmDecl *Parm) {
     const DefaultArgStorage &Storage = Parm->getDefaultArgStorage();
     if (auto *Prev = Storage.ValueOrInherited.template dyn_cast<ParmDecl*>())
       Parm = Prev;
-    assert(
-        Parm->getDefaultArgStorage().ValueOrInherited.template is<ArgType>() &&
-        "should only be one level of indirection");
+    assert(!Parm->getDefaultArgStorage()
+                .ValueOrInherited.template is<ParmDecl *>() &&
+           "should only be one level of indirection");
     return Parm;
   }
 
@@ -240,17 +257,24 @@ public:
   /// Determine whether the default argument for this parameter was inherited
   /// from a previous declaration of the same entity.
   bool isInherited() const { return ValueOrInherited.template is<ParmDecl*>(); }
-  /// Get the default argument's value.
+  /// Get the default argument's value. This does not consider whether the
+  /// default argument is visible.
   ArgType get() const {
     const DefaultArgStorage *Storage = this;
     if (auto *Prev = ValueOrInherited.template dyn_cast<ParmDecl*>())
       Storage = &Prev->getDefaultArgStorage();
+    if (auto *C = ValueOrInherited.template dyn_cast<Chain*>())
+      return C->Value;
     return Storage->ValueOrInherited.template get<ArgType>();
   }
   /// Get the parameter from which we inherit the default argument, if any.
   /// This is the parameter on which the default argument was actually written.
   const ParmDecl *getInheritedFrom() const {
-    return ValueOrInherited.template dyn_cast<ParmDecl*>();
+    if (auto *D = ValueOrInherited.template dyn_cast<ParmDecl*>())
+      return D;
+    if (auto *C = ValueOrInherited.template dyn_cast<Chain*>())
+      return C->PrevDeclWithDefaultArg;
+    return nullptr;
   }
   /// Set the default argument.
   void set(ArgType Arg) {
@@ -259,8 +283,16 @@ public:
   }
   /// Set that the default argument was inherited from another parameter.
   void setInherited(const ASTContext &C, ParmDecl *InheritedFrom) {
-    assert(!isSet() && "default argument already set");
-    ValueOrInherited = getParmOwningDefaultArg(InheritedFrom);
+    // Defined in DeclTemplate.cpp.
+    extern void *allocateDefaultArgStorageChain(const ASTContext &C);
+
+    assert(!isInherited() && "default argument already inherited");
+    InheritedFrom = getParmOwningDefaultArg(InheritedFrom);
+    if (!isSet())
+      ValueOrInherited = InheritedFrom;
+    else
+      ValueOrInherited = new (allocateDefaultArgStorageChain(C))
+          Chain{InheritedFrom, ValueOrInherited.template get<ArgType>()};
   }
   /// Remove the default argument, even if it was inherited.
   void clear() {

diff --git a/clang/include/clang/Sema/Lookup.h b/clang/include/clang/Sema/Lookup.h
index 97192b5..5bfee8b 100644 (file)
--- a/clang/include/clang/Sema/Lookup.h
+++ b/clang/include/clang/Sema/Lookup.h
@@ -302,14 +302,10 @@ public:
     if (!D->isInIdentifierNamespace(IDNS))
       return nullptr;
 
-    if (isVisible(getSema(), D))
+    if (isHiddenDeclarationVisible() || isVisible(getSema(), D))
       return D;
 
-    if (auto *Visible = getAcceptableDeclSlow(D))
-      return Visible;
-
-    // Even if hidden declarations are visible, prefer a visible declaration.
-    return isHiddenDeclarationVisible() ? D : nullptr;
+    return getAcceptableDeclSlow(D);
   }
 
 private:

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 60664c5..03ecf96 100644 (file)
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1325,6 +1325,9 @@ public:
     return hasVisibleDefinition(const_cast<NamedDecl*>(D), &Hidden);
   }
 
+  /// Determine if the template parameter \p D has a visible default argument.
+  bool hasVisibleDefaultArgument(const NamedDecl *D);
+
   bool RequireCompleteType(SourceLocation Loc, QualType T,
                            TypeDiagnoser &Diagnoser);
   bool RequireCompleteType(SourceLocation Loc, QualType T,

diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index c4e2c00..2544c85 100644 (file)
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -124,6 +124,12 @@ static void AdoptTemplateParameterList(TemplateParameterList *Params,
   }
 }
 
+namespace clang {
+void *allocateDefaultArgStorageChain(const ASTContext &C) {
+  return new (C) char[sizeof(void*) * 2];
+}
+}
+
 //===----------------------------------------------------------------------===//
 // RedeclarableTemplateDecl Implementation
 //===----------------------------------------------------------------------===//

diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index d0a55b5..147b81b 100644 (file)
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1228,6 +1228,8 @@ Module *Sema::getOwningModule(Decl *Entity) {
 }
 
 void Sema::makeMergedDefinitionVisible(NamedDecl *ND, SourceLocation Loc) {
+  // FIXME: If ND is a template declaration, make the template parameters
+  // visible too. They're not (necessarily) within its DeclContext.
   if (auto *M = PP.getModuleContainingLocation(Loc))
     Context.mergeDefinitionIntoModule(ND, M);
   else
@@ -1282,6 +1284,30 @@ bool Sema::hasVisibleMergedDefinition(NamedDecl *Def) {
   return false;
 }
 
+template<typename ParmDecl>
+static bool hasVisibleDefaultArgument(Sema &S, const ParmDecl *D) {
+  if (!D->hasDefaultArgument())
+    return false;
+
+  while (D) {
+    auto &DefaultArg = D->getDefaultArgStorage();
+    if (!DefaultArg.isInherited() && S.isVisible(D))
+      return true;
+
+    // If there was a previous default argument, maybe its parameter is visible.
+    D = DefaultArg.getInheritedFrom();
+  }
+  return false;
+}
+
+bool Sema::hasVisibleDefaultArgument(const NamedDecl *D) {
+  if (auto *P = dyn_cast<TemplateTypeParmDecl>(D))
+    return ::hasVisibleDefaultArgument(*this, P);
+  if (auto *P = dyn_cast<NonTypeTemplateParmDecl>(D))
+    return ::hasVisibleDefaultArgument(*this, P);
+  return ::hasVisibleDefaultArgument(*this, cast<TemplateTemplateParmDecl>(D));
+}
+
 /// \brief Determine whether a declaration is visible to name lookup.
 ///
 /// This routine determines whether the declaration D is visible in the current

diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 941672c..6bb4be1 100644 (file)
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1310,15 +1310,11 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
       // Merge default arguments for template type parameters.
       TemplateTypeParmDecl *OldTypeParm
           = OldParams? cast<TemplateTypeParmDecl>(*OldParam) : nullptr;
-      // FIXME: There might be a visible declaration of this template parameter.
-      if (OldTypeParm && !LookupResult::isVisible(*this, OldTypeParm))
-        OldTypeParm = nullptr;
-
       if (NewTypeParm->isParameterPack()) {
         assert(!NewTypeParm->hasDefaultArgument() &&
                "Parameter packs can't have a default argument!");
         SawParameterPack = true;
-      } else if (OldTypeParm && OldTypeParm->hasDefaultArgument() &&
+      } else if (OldTypeParm && hasVisibleDefaultArgument(OldTypeParm) &&
                  NewTypeParm->hasDefaultArgument()) {
         OldDefaultLoc = OldTypeParm->getDefaultArgumentLoc();
         NewDefaultLoc = NewTypeParm->getDefaultArgumentLoc();
@@ -1357,14 +1353,12 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
       // Merge default arguments for non-type template parameters
       NonTypeTemplateParmDecl *OldNonTypeParm
         = OldParams? cast<NonTypeTemplateParmDecl>(*OldParam) : nullptr;
-      if (OldNonTypeParm && !LookupResult::isVisible(*this, OldNonTypeParm))
-        OldNonTypeParm = nullptr;
       if (NewNonTypeParm->isParameterPack()) {
         assert(!NewNonTypeParm->hasDefaultArgument() &&
                "Parameter packs can't have a default argument!");
         if (!NewNonTypeParm->isPackExpansion())
           SawParameterPack = true;
-      } else if (OldNonTypeParm && OldNonTypeParm->hasDefaultArgument() &&
+      } else if (OldNonTypeParm && hasVisibleDefaultArgument(OldNonTypeParm) &&
                  NewNonTypeParm->hasDefaultArgument()) {
         OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc();
         NewDefaultLoc = NewNonTypeParm->getDefaultArgumentLoc();
@@ -1401,15 +1395,14 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
       // Merge default arguments for template template parameters
       TemplateTemplateParmDecl *OldTemplateParm
         = OldParams? cast<TemplateTemplateParmDecl>(*OldParam) : nullptr;
-      if (OldTemplateParm && !LookupResult::isVisible(*this, OldTemplateParm))
-        OldTemplateParm = nullptr;
       if (NewTemplateParm->isParameterPack()) {
         assert(!NewTemplateParm->hasDefaultArgument() &&
                "Parameter packs can't have a default argument!");
         if (!NewTemplateParm->isPackExpansion())
           SawParameterPack = true;
-      } else if (OldTemplateParm && OldTemplateParm->hasDefaultArgument() &&
-          NewTemplateParm->hasDefaultArgument()) {
+      } else if (OldTemplateParm &&
+                 hasVisibleDefaultArgument(OldTemplateParm) &&
+                 NewTemplateParm->hasDefaultArgument()) {
         OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation();
         NewDefaultLoc = NewTemplateParm->getDefaultArgument().getLocation();
         SawDefaultArgument = true;

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index af125cf..9cb145e 100644 (file)
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2908,8 +2908,7 @@ static bool inheritDefaultTemplateArgument(ASTContext &Context, ParmDecl *From,
   auto *To = cast<ParmDecl>(ToD);
   if (!From->hasDefaultArgument())
     return false;
-  if (!To->hasDefaultArgument())
-    To->setInheritedDefaultArgument(Context, From);
+  To->setInheritedDefaultArgument(Context, From);
   return true;
 }
 

diff --git a/clang/test/Modules/Inputs/template-default-args/a.h b/clang/test/Modules/Inputs/template-default-args/a.h
index 1ef1ea5..3e41535 100644 (file)
--- a/clang/test/Modules/Inputs/template-default-args/a.h
+++ b/clang/test/Modules/Inputs/template-default-args/a.h
@@ -2,3 +2,4 @@ template<typename T = int> struct A {};
 template<typename T> struct B {};
 template<typename T> struct C;
 template<typename T> struct D;
+template<typename T> struct E;

diff --git a/clang/test/Modules/Inputs/template-default-args/c.h b/clang/test/Modules/Inputs/template-default-args/c.h
new file mode 100644 (file)
index 0000000..c204f31
--- /dev/null
+++ b/clang/test/Modules/Inputs/template-default-args/c.h
@@ -0,0 +1 @@
+template<typename T = int> struct F;

diff --git a/clang/test/Modules/Inputs/template-default-args/module.modulemap b/clang/test/Modules/Inputs/template-default-args/module.modulemap
index 6182e6b..d54dfc3 100644 (file)
--- a/clang/test/Modules/Inputs/template-default-args/module.modulemap
+++ b/clang/test/Modules/Inputs/template-default-args/module.modulemap
@@ -1 +1,5 @@
-module X { module A { header "a.h" } module B { header "b.h" } }
+module X {
+  module A { header "a.h" }
+  module B { header "b.h" }
+  module C { header "c.h" }
+}

diff --git a/clang/test/Modules/template-default-args.cpp b/clang/test/Modules/template-default-args.cpp
index 63187b8..97569f0 100644 (file)
--- a/clang/test/Modules/template-default-args.cpp
+++ b/clang/test/Modules/template-default-args.cpp
@@ -7,6 +7,7 @@ template<typename T> struct A;
 template<typename T> struct B;
 template<typename T> struct C;
 template<typename T = int> struct D;
+template<typename T = int> struct E {};
 
 #include "b.h"
 
@@ -15,8 +16,13 @@ template<typename T> struct B {};
 template<typename T = int> struct B;
 template<typename T = int> struct C;
 template<typename T> struct D {};
+template<typename T> struct F {};
+
+#include "c.h"
 
 A<> a;
 B<> b;
 extern C<> c;
 D<> d;
+E<> e;
+F<> f;