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

argument is not visible.

llvm-svn: 239934

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7636,12 +7636,12 @@ def err_module_private_local : Error<
 def err_module_private_local_class : Error<
   "local %select{struct|interface|union|class|enum}0 cannot be declared "
   "__module_private__">;
-def err_module_private_declaration : Error<
+def err_module_unimported_use : Error<
-  "%select{declaration|definition}0 of %1 must be imported from "
+  "%select{declaration|definition|default argument}0 of %1 must be imported "
-  "module '%2' before it is required">;
+  "from module '%2' before it is required">;
-def err_module_private_declaration_multiple : Error<
+def err_module_unimported_use_multiple : Error<
-  "%select{declaration|definition}0 of %1 must be imported from "
+  "%select{declaration|definition|default argument}0 of %1 must be imported "
-  "one of the following modules before it is required:%2">;
+  "from one of the following modules before it is required:%2">;
 def err_module_import_in_extern_c : Error<
   "import of C++ module '%0' appears within extern \"C\" language linkage "
   "specification">;

clang/include/clang/Sema/Sema.h

@@ -1326,7 +1326,9 @@ public:
   }
 
   /// Determine if the template parameter \p D has a visible default argument.
-  bool hasVisibleDefaultArgument(const NamedDecl *D);
+  bool
+  hasVisibleDefaultArgument(const NamedDecl *D,
+                            llvm::SmallVectorImpl<Module *> *Modules = nullptr);
 
   bool RequireCompleteType(SourceLocation Loc, QualType T,
                            TypeDiagnoser &Diagnoser);
@@ -1730,10 +1732,21 @@ public:
   void createImplicitModuleImportForErrorRecovery(SourceLocation Loc,
                                                   Module *Mod);
 
+  /// Kinds of missing import. Note, the values of these enumerators correspond
+  /// to %select values in diagnostics.
+  enum class MissingImportKind {
+    Declaration,
+    Definition,
+    DefaultArgument
+  };
+
   /// \brief Diagnose that the specified declaration needs to be visible but
   /// isn't, and suggest a module import that would resolve the problem.
   void diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
                              bool NeedDefinition, bool Recover = true);
+  void diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl,
+                             SourceLocation DeclLoc, ArrayRef<Module *> Modules,
+                             MissingImportKind MIK, bool Recover);
 
   /// \brief Retrieve a suitable printing policy.
   PrintingPolicy getPrintingPolicy() const {

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())

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, P, Modules);
-  return ::hasVisibleDefaultArgument(*this, cast<TemplateTemplateParmDecl>(D));
+  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)
+    Diag(UseLoc, diag::err_module_unimported_use_multiple)
-      << NeedDefinition << Decl << ModuleList;
+      << (int)MIK << Decl << ModuleList;
   } else {
-    Diag(Loc, diag::err_module_private_declaration)
+    Diag(UseLoc, diag::err_module_unimported_use)
-      << NeedDefinition << Decl << Owner->getFullModuleName();
+      << (int)MIK << Decl << Modules[0]->getFullModuleName();
   }
-  Diag(Decl->getLocation(), NeedDefinition ? diag::note_previous_definition
+
-                                           : diag::note_previous_declaration);
+  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(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

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,

clang/test/Modules/submodules-merge-defs.cpp

@@ -48,7 +48,15 @@ int pre_ff = F<int>().f(); // expected-error +{{must be imported}}
 int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}}
 // expected-note@defs.h:26 +{{here}}
 
-J<> pre_j; // expected-error {{must be imported}} expected-error {{too few}}
+J<> pre_j; // expected-error {{declaration of 'J' must be imported}}
+#ifdef IMPORT_USE_2
+// FIXME-error-re@-2 {{default argument of 'J' must be imported from one of {{.*}}stuff.use{{.*}}stuff.use-2}}
+// expected-error@-3 {{default argument of 'J' must be imported from module 'stuff.use'}}
+#elif EARLY_INDIRECT_INCLUDE
+// expected-error@-5 {{default argument of 'J' must be imported from module 'merged-defs'}}
+#else
+// expected-error@-7 {{default argument of 'J' must be imported from module 'stuff.use'}}
+#endif
 // expected-note@defs.h:49 +{{here}}
 
 // Make definitions from second module visible.

clang/test/Modules/template-default-args.cpp

@@ -1,12 +1,12 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -I %S/Inputs/template-default-args -std=c++11 %s
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -verify -fmodules-cache-path=%t -fno-modules-error-recovery -I %S/Inputs/template-default-args -std=c++11 %s
 
 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 {};
-template<typename T> struct H {}; // expected-note {{here}}
+template<typename T> struct H {};
 
 #include "b.h"
 
@@ -16,7 +16,7 @@ template<typename T = int> struct B;
 template<typename T = int> struct C;
 template<typename T> struct D {};
 template<typename T> struct F {};
-template<typename T> struct G {}; // expected-note {{here}}
+template<typename T> struct G {};
 
 #include "c.h"
 
@@ -26,5 +26,7 @@ extern C<> c;
 D<> d;
 E<> e;
 F<> f;
-G<> g; // expected-error {{too few}}
+G<> g; // expected-error {{default argument of 'G' must be imported from module 'X.A' before it is required}}
-H<> h; // expected-error {{too few}}
+// expected-note@a.h:6 {{default argument declared here}}
+H<> h; // expected-error {{default argument of 'H' must be imported from module 'X.A' before it is required}}
+// expected-note@a.h:7 {{default argument declared here}}