[modules] Improve diagnostic for a template-id that's invalid because a default

argument is not visible.

llvm-svn: 239934

3 files changed, 90 insertions, 20 deletions

diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 2544c85bbd3..cde497b012e 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -666,6 +666,11 @@ TemplateTemplateParmDecl::CreateDeserialized(ASTContext &C, unsigned ID,
                                nullptr, NumExpansions, nullptr);
 }
 
+SourceLocation TemplateTemplateParmDecl::getDefaultArgumentLoc() const {
+  return hasDefaultArgument() ? getDefaultArgument().getLocation()
+                              : SourceLocation();
+}
+
 void TemplateTemplateParmDecl::setDefaultArgument(
     const ASTContext &C, const TemplateArgumentLoc &DefArg) {
   if (DefArg.getArgument().isNull())
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index b5ef3a42d17..75d4709e19b 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1285,7 +1285,9 @@ bool Sema::hasVisibleMergedDefinition(NamedDecl *Def) {
 }
 
 template<typename ParmDecl>
-static bool hasVisibleDefaultArgument(Sema &S, const ParmDecl *D) {
+static bool
+hasVisibleDefaultArgument(Sema &S, const ParmDecl *D,
+                          llvm::SmallVectorImpl<Module *> *Modules) {
   if (!D->hasDefaultArgument())
     return false;
 
@@ -1294,18 +1296,23 @@ static bool hasVisibleDefaultArgument(Sema &S, const ParmDecl *D) {
     if (!DefaultArg.isInherited() && S.isVisible(D))
       return true;
 
+    if (!DefaultArg.isInherited() && Modules)
+      Modules->push_back(S.getOwningModule(const_cast<ParmDecl*>(D)));
+
     // If there was a previous default argument, maybe its parameter is visible.
     D = DefaultArg.getInheritedFrom();
   }
   return false;
 }
 
-bool Sema::hasVisibleDefaultArgument(const NamedDecl *D) {
+bool Sema::hasVisibleDefaultArgument(const NamedDecl *D,
+                                     llvm::SmallVectorImpl<Module *> *Modules) {
   if (auto *P = dyn_cast<TemplateTypeParmDecl>(D))
-    return ::hasVisibleDefaultArgument(*this, P);
+    return ::hasVisibleDefaultArgument(*this, P, Modules);
   if (auto *P = dyn_cast<NonTypeTemplateParmDecl>(D))
-    return ::hasVisibleDefaultArgument(*this, P);
-  return ::hasVisibleDefaultArgument(*this, cast<TemplateTemplateParmDecl>(D));
+    return ::hasVisibleDefaultArgument(*this, P, Modules);
+  return ::hasVisibleDefaultArgument(*this, cast<TemplateTemplateParmDecl>(D),
+                                     Modules);
 }
 
 /// \brief Determine whether a declaration is visible to name lookup.
@@ -4676,33 +4683,59 @@ void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
   Module *Owner = getOwningModule(Decl);
   assert(Owner && "definition of hidden declaration is not in a module");
 
+  llvm::SmallVector<Module*, 8> OwningModules;
+  OwningModules.push_back(Owner);
   auto Merged = Context.getModulesWithMergedDefinition(Decl);
-  if (!Merged.empty()) {
+  OwningModules.insert(OwningModules.end(), Merged.begin(), Merged.end());
+
+  diagnoseMissingImport(Loc, Decl, Decl->getLocation(), OwningModules,
+                        NeedDefinition ? MissingImportKind::Definition
+                                       : MissingImportKind::Declaration,
+                        Recover);
+}
+
+void Sema::diagnoseMissingImport(SourceLocation UseLoc, NamedDecl *Decl,
+                                 SourceLocation DeclLoc,
+                                 ArrayRef<Module *> Modules,
+                                 MissingImportKind MIK, bool Recover) {
+  assert(!Modules.empty());
+
+  if (Modules.size() > 1) {
     std::string ModuleList;
-    ModuleList += "\n        ";
-    ModuleList += Owner->getFullModuleName();
     unsigned N = 0;
-    for (Module *M : Merged) {
+    for (Module *M : Modules) {
       ModuleList += "\n        ";
-      if (++N == 5 && Merged.size() != N) {
+      if (++N == 5 && N != Modules.size()) {
         ModuleList += "[...]";
         break;
       }
       ModuleList += M->getFullModuleName();
     }
 
-    Diag(Loc, diag::err_module_private_declaration_multiple)
-      << NeedDefinition << Decl << ModuleList;
+    Diag(UseLoc, diag::err_module_unimported_use_multiple)
+      << (int)MIK << Decl << ModuleList;
   } else {
-    Diag(Loc, diag::err_module_private_declaration)
-      << NeedDefinition << Decl << Owner->getFullModuleName();
+    Diag(UseLoc, diag::err_module_unimported_use)
+      << (int)MIK << Decl << Modules[0]->getFullModuleName();
+  }
+
+  unsigned DiagID;
+  switch (MIK) {
+  case MissingImportKind::Declaration:
+    DiagID = diag::note_previous_declaration;
+    break;
+  case MissingImportKind::Definition:
+    DiagID = diag::note_previous_definition;
+    break;
+  case MissingImportKind::DefaultArgument:
+    DiagID = diag::note_default_argument_declared_here;
+    break;
   }
-  Diag(Decl->getLocation(), NeedDefinition ? diag::note_previous_definition
-                                           : diag::note_previous_declaration);
+  Diag(DeclLoc, DiagID);
 
   // Try to recover by implicitly importing this module.
   if (Recover)
-    createImplicitModuleImportForErrorRecovery(Loc, Owner);
+    createImplicitModuleImportForErrorRecovery(UseLoc, Modules[0]);
 }
 
 /// \brief Diagnose a successfully-corrected typo. Separated from the correction
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 9a154111226..f4740a5cd85 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -3644,6 +3644,35 @@ static Optional<unsigned> getExpandedPackSize(NamedDecl *Param) {
   return None;
 }
 
+/// Diagnose a missing template argument.
+template<typename TemplateParmDecl>
+static bool diagnoseMissingArgument(Sema &S, SourceLocation Loc,
+                                    TemplateDecl *TD,
+                                    const TemplateParmDecl *D,
+                                    TemplateArgumentListInfo &Args) {
+  // Dig out the most recent declaration of the template parameter; there may be
+  // declarations of the template that are more recent than TD.
+  D = cast<TemplateParmDecl>(cast<TemplateDecl>(TD->getMostRecentDecl())
+                                 ->getTemplateParameters()
+                                 ->getParam(D->getIndex()));
+
+  // If there's a default argument that's not visible, diagnose that we're
+  // missing a module import.
+  llvm::SmallVector<Module*, 8> Modules;
+  if (D->hasDefaultArgument() && !S.hasVisibleDefaultArgument(D, &Modules)) {
+    S.diagnoseMissingImport(Loc, cast<NamedDecl>(TD),
+                            D->getDefaultArgumentLoc(), Modules,
+                            Sema::MissingImportKind::DefaultArgument,
+                            /*Recover*/ true);
+    return true;
+  }
+
+  // FIXME: If there's a more recent default argument that *is* visible,
+  // diagnose that it was declared too late.
+
+  return diagnoseArityMismatch(S, TD, Loc, Args);
+}
+
 /// \brief Check that the given template argument list is well-formed
 /// for specializing the given template.
 bool Sema::CheckTemplateArgumentList(TemplateDecl *Template,
@@ -3800,7 +3829,8 @@ bool Sema::CheckTemplateArgumentList(TemplateDecl *Template,
     // the default argument.
     if (TemplateTypeParmDecl *TTP = dyn_cast<TemplateTypeParmDecl>(*Param)) {
       if (!hasVisibleDefaultArgument(TTP))
-        return diagnoseArityMismatch(*this, Template, TemplateLoc, NewArgs);
+        return diagnoseMissingArgument(*this, TemplateLoc, Template, TTP,
+                                       NewArgs);
 
       TypeSourceInfo *ArgType = SubstDefaultTemplateArgument(*this,
                                                              Template,
@@ -3816,7 +3846,8 @@ bool Sema::CheckTemplateArgumentList(TemplateDecl *Template,
     } else if (NonTypeTemplateParmDecl *NTTP
                  = dyn_cast<NonTypeTemplateParmDecl>(*Param)) {
       if (!hasVisibleDefaultArgument(NTTP))
-        return diagnoseArityMismatch(*this, Template, TemplateLoc, NewArgs);
+        return diagnoseMissingArgument(*this, TemplateLoc, Template, NTTP,
+                                       NewArgs);
 
       ExprResult E = SubstDefaultTemplateArgument(*this, Template,
                                                               TemplateLoc,
@@ -3833,7 +3864,8 @@ bool Sema::CheckTemplateArgumentList(TemplateDecl *Template,
         = cast<TemplateTemplateParmDecl>(*Param);
 
       if (!hasVisibleDefaultArgument(TempParm))
-        return diagnoseArityMismatch(*this, Template, TemplateLoc, NewArgs);
+        return diagnoseMissingArgument(*this, TemplateLoc, Template, TempParm,
+                                       NewArgs);
 
       NestedNameSpecifierLoc QualifierLoc;
       TemplateName Name = SubstDefaultTemplateArgument(*this, Template,