From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Wed, 10 Jun 2015 20:30:23 +0000 (+0000)
Subject: [modules] Track all default template arguments for a given parameter across
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fc970014c204eae2421c814bf240999b105b01fc;p=clang

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


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@239485 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/AST/DeclTemplate.h b/include/clang/AST/DeclTemplate.h
index 324f0e8058..d4e090eb81 100644
--- a/include/clang/AST/DeclTemplate.h
+++ b/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/include/clang/Sema/Lookup.h b/include/clang/Sema/Lookup.h
index 97192b53fa..5bfee8b0d0 100644
--- a/include/clang/Sema/Lookup.h
+++ b/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/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 60664c5fdc..03ecf96ea3 100644
--- a/include/clang/Sema/Sema.h
+++ b/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/lib/AST/DeclTemplate.cpp b/lib/AST/DeclTemplate.cpp
index c4e2c007c1..2544c85bbd 100644
--- a/lib/AST/DeclTemplate.cpp
+++ b/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/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index d0a55b57c6..147b81bc59 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/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/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp
index 941672c6e3..6bb4be1d18 100644
--- a/lib/Sema/SemaTemplate.cpp
+++ b/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/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp
index af125cf660..9cb145e4b4 100644
--- a/lib/Serialization/ASTReaderDecl.cpp
+++ b/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/test/Modules/Inputs/template-default-args/a.h b/test/Modules/Inputs/template-default-args/a.h
index 1ef1ea5907..3e41535824 100644
--- a/test/Modules/Inputs/template-default-args/a.h
+++ b/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/test/Modules/Inputs/template-default-args/c.h b/test/Modules/Inputs/template-default-args/c.h
new file mode 100644
index 0000000000..c204f31336
--- /dev/null
+++ b/test/Modules/Inputs/template-default-args/c.h
@@ -0,0 +1 @@
+template<typename T = int> struct F;
diff --git a/test/Modules/Inputs/template-default-args/module.modulemap b/test/Modules/Inputs/template-default-args/module.modulemap
index 6182e6b3ee..d54dfc345a 100644
--- a/test/Modules/Inputs/template-default-args/module.modulemap
+++ b/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/test/Modules/template-default-args.cpp b/test/Modules/template-default-args.cpp
index 63187b8dc1..97569f0aa3 100644
--- a/test/Modules/template-default-args.cpp
+++ b/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;