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@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. 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@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}} _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits