r239934 - [modules] Improve diagnostic for a template-id that's invalid because a default
Richard Smith
richard-llvm at metafoo.co.uk
Wed Jun 17 13:16:32 PDT 2015
Author: rsmith
Date: Wed Jun 17 15:16:32 2015
New Revision: 239934
URL: http://llvm.org/viewvc/llvm-project?rev=239934&view=rev
Log:
[modules] Improve diagnostic for a template-id that's invalid because a default
argument is not visible.
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/DeclTemplate.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/Modules/submodules-merge-defs.cpp
cfe/trunk/test/Modules/template-default-args.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=239934&r1=239933&r2=239934&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jun 17 15:16:32 2015
@@ -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<
- "%select{declaration|definition}0 of %1 must be imported from "
- "module '%2' before it is required">;
-def err_module_private_declaration_multiple : Error<
- "%select{declaration|definition}0 of %1 must be imported from "
- "one of the following modules before it is required:%2">;
+def err_module_unimported_use : Error<
+ "%select{declaration|definition|default argument}0 of %1 must be imported "
+ "from module '%2' before it is required">;
+def err_module_unimported_use_multiple : Error<
+ "%select{declaration|definition|default argument}0 of %1 must be imported "
+ "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">;
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=239934&r1=239933&r2=239934&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Jun 17 15:16:32 2015
@@ -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 {
Modified: cfe/trunk/lib/AST/DeclTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclTemplate.cpp?rev=239934&r1=239933&r2=239934&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclTemplate.cpp (original)
+++ cfe/trunk/lib/AST/DeclTemplate.cpp Wed Jun 17 15:16:32 2015
@@ -666,6 +666,11 @@ TemplateTemplateParmDecl::CreateDeserial
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())
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=239934&r1=239933&r2=239934&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed Jun 17 15:16:32 2015
@@ -1285,7 +1285,9 @@ bool Sema::hasVisibleMergedDefinition(Na
}
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(Se
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(SourceL
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
Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=239934&r1=239933&r2=239934&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Wed Jun 17 15:16:32 2015
@@ -3644,6 +3644,35 @@ static Optional<unsigned> getExpandedPac
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(Tem
// 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(Tem
} 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(Tem
= 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,
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=239934&r1=239933&r2=239934&view=diff
==============================================================================
--- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original)
+++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Wed Jun 17 15:16:32 2015
@@ -48,7 +48,15 @@ int pre_ff = F<int>().f(); // expected-e
int pre_fg = F<int>().g<int>(); // expected-error +{{must be imported}}
// expected-note at 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 at -2 {{default argument of 'J' must be imported from one of {{.*}}stuff.use{{.*}}stuff.use-2}}
+// expected-error at -3 {{default argument of 'J' must be imported from module 'stuff.use'}}
+#elif EARLY_INDIRECT_INCLUDE
+// expected-error at -5 {{default argument of 'J' must be imported from module 'merged-defs'}}
+#else
+// expected-error at -7 {{default argument of 'J' must be imported from module 'stuff.use'}}
+#endif
// expected-note at defs.h:49 +{{here}}
// Make definitions from second module visible.
Modified: cfe/trunk/test/Modules/template-default-args.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/template-default-args.cpp?rev=239934&r1=239933&r2=239934&view=diff
==============================================================================
--- cfe/trunk/test/Modules/template-default-args.cpp (original)
+++ cfe/trunk/test/Modules/template-default-args.cpp Wed Jun 17 15:16:32 2015
@@ -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}}
-H<> h; // expected-error {{too few}}
+G<> g; // expected-error {{default argument of 'G' must be imported from module 'X.A' before it is required}}
+// expected-note at 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 at a.h:7 {{default argument declared here}}
More information about the cfe-commits
mailing list