From: Richard Smith Date: Wed, 7 May 2014 02:25:43 +0000 (+0000) Subject: If an instantiation of a template is required to be a complete type, check X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=67198ad8f5872a5b29ce440a1388a70dca437d95;p=clang If an instantiation of a template is required to be a complete type, check whether the definition of the template is visible rather than checking whether the instantiated definition happens to be in an imported module. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@208150 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/LangOptions.def b/include/clang/Basic/LangOptions.def index 699bf1fa4d..82f7094a2a 100644 --- a/include/clang/Basic/LangOptions.def +++ b/include/clang/Basic/LangOptions.def @@ -98,6 +98,7 @@ LANGOPT(Modules , 1, 0, "modules extension to C") LANGOPT(ModulesDeclUse , 1, 0, "require declaration of module uses") LANGOPT(ModulesSearchAll , 1, 1, "search even non-imported modules to find unresolved references") LANGOPT(ModulesStrictDeclUse, 1, 0, "require declaration of module uses and all headers to be in modules") +LANGOPT(ModulesErrorRecovery, 1, 1, "automatically import modules as needed when performing error recovery") LANGOPT(Optimize , 1, 0, "__OPTIMIZE__ predefined macro") LANGOPT(OptimizeSize , 1, 0, "__OPTIMIZE_SIZE__ predefined macro") LANGOPT(Static , 1, 0, "__STATIC__ predefined macro (as opposed to __DYNAMIC__)") diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td index e84a20b569..b33198d556 100644 --- a/include/clang/Driver/CC1Options.td +++ b/include/clang/Driver/CC1Options.td @@ -314,6 +314,8 @@ def ast_dump_lookups : Flag<["-"], "ast-dump-lookups">, HelpText<"Include name lookup table dumps in AST dumps">; def fno_modules_global_index : Flag<["-"], "fno-modules-global-index">, HelpText<"Do not automatically generate or update the global module index">; +def fno_modules_error_recovery : Flag<["-"], "fno-modules-error-recovery">, + HelpText<"Do not automatically import modules for error recovery">; let Group = Action_Group in { diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 6047acbd8b..ced85fc278 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1708,12 +1708,13 @@ public: void ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod); /// \brief Create an implicit import of the given module at the given - /// source location. + /// source location, for error recovery, if possible. /// - /// This routine is typically used for error recovery, when the entity found - /// by name lookup is actually hidden within a module that we know about but - /// the user has forgotten to import. - void createImplicitModuleImport(SourceLocation Loc, Module *Mod); + /// This routine is typically used when an entity found by name lookup + /// is actually hidden within a module that we know about but the user + /// has forgotten to import. + void createImplicitModuleImportForErrorRecovery(SourceLocation Loc, + Module *Mod); /// \brief Retrieve a suitable printing policy. PrintingPolicy getPrintingPolicy() const { diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index f499ae0f83..0b6b765dba 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1359,6 +1359,7 @@ static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK, Opts.ModulesSearchAll = Opts.Modules && !Args.hasArg(OPT_fno_modules_search_all) && Args.hasArg(OPT_fmodules_search_all); + Opts.ModulesErrorRecovery = !Args.hasArg(OPT_fno_modules_error_recovery); Opts.CharIsSigned = Opts.OpenCL || !Args.hasArg(OPT_fno_signed_char); Opts.WChar = Opts.CPlusPlus && !Args.hasArg(OPT_fno_wchar); Opts.ShortWChar = Args.hasFlag(OPT_fshort_wchar, OPT_fno_short_wchar, false); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 3ee1577b0b..0d448e0aa8 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -13177,7 +13177,12 @@ void Sema::ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod) { /*Complain=*/true); } -void Sema::createImplicitModuleImport(SourceLocation Loc, Module *Mod) { +void Sema::createImplicitModuleImportForErrorRecovery(SourceLocation Loc, + Module *Mod) { + // Bail if we're not allowed to implicitly import a module here. + if (isSFINAEContext() || !getLangOpts().ModulesErrorRecovery) + return; + // Create the implicit import declaration. TranslationUnitDecl *TU = getASTContext().getTranslationUnitDecl(); ImportDecl *ImportD = ImportDecl::CreateImplicit(getASTContext(), TU, diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index db7fe6e614..fc07e7273f 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -4577,9 +4577,9 @@ void Sema::diagnoseTypo(const TypoCorrection &Correction, Diag(Def->getLocation(), diag::note_previous_declaration); // Recover by implicitly importing this module. - if (!isSFINAEContext() && ErrorRecovery) - createImplicitModuleImport(Correction.getCorrectionRange().getBegin(), - Owner); + if (ErrorRecovery) + createImplicitModuleImportForErrorRecovery( + Correction.getCorrectionRange().getBegin(), Owner); return; } diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 3f03d43f79..0afbe01586 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -5076,6 +5076,69 @@ bool Sema::RequireCompleteType(SourceLocation Loc, QualType T, return false; } +/// \brief Determine whether there is any declaration of \p D that was ever a +/// definition (perhaps before module merging) and is currently visible. +/// \param D The definition of the entity. +/// \param Suggested Filled in with the declaration that should be made visible +/// in order to provide a definition of this entity. +static bool hasVisibleDefinition(Sema &S, NamedDecl *D, NamedDecl **Suggested) { + // Easy case: if we don't have modules, all declarations are visible. + if (!S.getLangOpts().Modules) + return true; + + // If this definition was instantiated from a template, map back to the + // pattern from which it was instantiated. + // + // FIXME: There must be a better place for this to live. + if (auto *RD = dyn_cast(D)) { + if (auto *TD = dyn_cast(RD)) { + auto From = TD->getInstantiatedFrom(); + if (auto *CTD = From.dyn_cast()) { + while (auto *NewCTD = CTD->getInstantiatedFromMemberTemplate()) { + if (NewCTD->isMemberSpecialization()) + break; + CTD = NewCTD; + } + RD = CTD->getTemplatedDecl(); + } else if (auto *CTPSD = From.dyn_cast< + ClassTemplatePartialSpecializationDecl *>()) { + while (auto *NewCTPSD = CTPSD->getInstantiatedFromMember()) { + if (NewCTPSD->isMemberSpecialization()) + break; + CTPSD = NewCTPSD; + } + RD = CTPSD; + } + } else if (isTemplateInstantiation(RD->getTemplateSpecializationKind())) { + while (auto *NewRD = RD->getInstantiatedFromMemberClass()) + RD = NewRD; + } + D = RD->getDefinition(); + } else if (auto *ED = dyn_cast(D)) { + while (auto *NewED = ED->getInstantiatedFromMemberEnum()) + ED = NewED; + if (ED->isFixed()) { + // If the enum has a fixed underlying type, any declaration of it will do. + *Suggested = 0; + for (auto *Redecl : ED->redecls()) { + if (LookupResult::isVisible(S, Redecl)) + return true; + if (Redecl->isThisDeclarationADefinition() || + (Redecl->isCanonicalDecl() && !*Suggested)) + *Suggested = Redecl; + } + return false; + } + D = ED->getDefinition(); + } + assert(D && "missing definition for pattern of instantiated definition"); + + // FIXME: If we merged any other decl into D, and that declaration is visible, + // then we should consider a definition to be visible. + *Suggested = D; + return LookupResult::isVisible(S, D); +} + /// \brief The implementation of RequireCompleteType bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, TypeDiagnoser &Diagnoser) { @@ -5091,21 +5154,21 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, NamedDecl *Def = 0; if (!T->isIncompleteType(&Def)) { // If we know about the definition but it is not visible, complain. - if (!Diagnoser.Suppressed && Def && !LookupResult::isVisible(*this, Def)) { + NamedDecl *SuggestedDef = 0; + if (!Diagnoser.Suppressed && Def && + !hasVisibleDefinition(*this, Def, &SuggestedDef)) { // Suppress this error outside of a SFINAE context if we've already // emitted the error once for this type. There's no usefulness in // repeating the diagnostic. // FIXME: Add a Fix-It that imports the corresponding module or includes // the header. - Module *Owner = Def->getOwningModule(); + Module *Owner = SuggestedDef->getOwningModule(); Diag(Loc, diag::err_module_private_definition) << T << Owner->getFullModuleName(); - Diag(Def->getLocation(), diag::note_previous_definition); + Diag(SuggestedDef->getLocation(), diag::note_previous_definition); - if (!isSFINAEContext()) { - // Recover by implicitly importing this module. - createImplicitModuleImport(Loc, Owner); - } + // Try to recover by implicitly importing this module. + createImplicitModuleImportForErrorRecovery(Loc, Owner); } // We lock in the inheritance model once somebody has asked us to ensure diff --git a/test/Modules/Inputs/cxx-templates-b-impl.h b/test/Modules/Inputs/cxx-templates-b-impl.h index fdf4a4fbc4..93d057433d 100644 --- a/test/Modules/Inputs/cxx-templates-b-impl.h +++ b/test/Modules/Inputs/cxx-templates-b-impl.h @@ -3,3 +3,10 @@ struct DefinedInBImpl { struct Inner {}; friend void FoundByADL(DefinedInBImpl); }; + +@import cxx_templates_common; +template struct TemplateInstantiationVisibility; +extern template struct TemplateInstantiationVisibility; +template<> struct TemplateInstantiationVisibility {}; +extern TemplateInstantiationVisibility::type + TemplateInstantiationVisibility_ImplicitInstantiation; diff --git a/test/Modules/Inputs/cxx-templates-common.h b/test/Modules/Inputs/cxx-templates-common.h index 9b46539a99..efbda2bd25 100644 --- a/test/Modules/Inputs/cxx-templates-common.h +++ b/test/Modules/Inputs/cxx-templates-common.h @@ -21,3 +21,5 @@ namespace Std { extern T g(); } } + +template struct TemplateInstantiationVisibility { typedef int type; }; diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp index 82ecca31b5..ab65e3dd2d 100644 --- a/test/Modules/cxx-templates.cpp +++ b/test/Modules/cxx-templates.cpp @@ -1,8 +1,8 @@ // RUN: rm -rf %t -// RUN: not %clang_cc1 -x objective-c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump -ast-dump-lookups | FileCheck %s --check-prefix=CHECK-GLOBAL -// RUN: not %clang_cc1 -x objective-c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump -ast-dump-lookups -ast-dump-filter N | FileCheck %s --check-prefix=CHECK-NAMESPACE-N -// RUN: not %clang_cc1 -x objective-c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump | FileCheck %s --check-prefix=CHECK-DUMP -// RUN: %clang_cc1 -x objective-c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 +// RUN: not %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump -ast-dump-lookups | FileCheck %s --check-prefix=CHECK-GLOBAL +// RUN: not %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump -ast-dump-lookups -ast-dump-filter N | FileCheck %s --check-prefix=CHECK-NAMESPACE-N +// RUN: not %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -std=c++11 -ast-dump | FileCheck %s --check-prefix=CHECK-DUMP +// RUN: %clang_cc1 -x objective-c++ -fmodules -fno-modules-error-recovery -fmodules-cache-path=%t -I %S/Inputs %s -verify -std=c++11 @import cxx_templates_a; @import cxx_templates_b; @@ -71,8 +71,15 @@ void g() { // Trigger the instantiation of a template in 'a' that uses a type defined in // 'b_impl'. That type is not visible here, nor in 'a'. This fails; there is // no reason why DefinedInBImpl should be visible here. + // + // We turn off error recovery for modules in this test (so we don't get an + // implicit import of cxx_templates_b_impl), and that results in us producing + // a big spew of errors here. + // // expected-error@Inputs/cxx-templates-a.h:19 {{definition of 'DefinedInBImpl' must be imported}} - // expected-note@Inputs/cxx-templates-b-impl.h:1 {{definition is here}} + // expected-note@Inputs/cxx-templates-b-impl.h:1 +{{definition is here}} + // expected-error@Inputs/cxx-templates-a.h:19 +{{}} + // expected-error@Inputs/cxx-templates-a.h:20 +{{}} PerformDelayedLookup(defined_in_b_impl); // expected-note {{in instantiation of}} merge_templates_a = merge_templates_b; // ok, same type @@ -89,6 +96,12 @@ void g() { static_assert(enum_c_from_a == enum_c_from_b, ""); CommonTemplate cti; CommonTemplate::E eee = CommonTemplate::c; + + TemplateInstantiationVisibility tiv1; + TemplateInstantiationVisibility tiv2; + TemplateInstantiationVisibility tiv3; // expected-error {{must be imported from module 'cxx_templates_b_impl'}} + // expected-note@cxx-templates-b-impl.h:10 {{previous definition is here}} + TemplateInstantiationVisibility tiv4; } RedeclaredAsFriend raf1; diff --git a/test/Modules/template-specialization-visibility.cpp b/test/Modules/template-specialization-visibility.cpp index cc11a173e1..efcfd93dd8 100644 --- a/test/Modules/template-specialization-visibility.cpp +++ b/test/Modules/template-specialization-visibility.cpp @@ -1,43 +1,26 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -fmodules -verify -fmodules-cache-path=%t -I %S/Inputs/template-specialization-visibility -std=c++11 %s // -// FIXME: We should accept the explicit instantiation cases below too. -// Note, errors trigger implicit imports, so only enable one error at a time. -// RUN: %clang_cc1 -fmodules -verify -fmodules-cache-path=%t -I %S/Inputs/template-specialization-visibility -std=c++11 -DERR1 %s -// RUN: %clang_cc1 -fmodules -verify -fmodules-cache-path=%t -I %S/Inputs/template-specialization-visibility -std=c++11 -DERR2 %s -// RUN: %clang_cc1 -fmodules -verify -fmodules-cache-path=%t -I %S/Inputs/template-specialization-visibility -std=c++11 -DERR3 %s +// expected-no-diagnostics #include "c.h" S implicit_inst_class_template; int k1 = implicit_inst_class_template.n; -#ifdef ERR1 -S explicit_inst_class_template; // expected-error {{must be imported from module 'tsv.e'}} -// expected-note@e.h:4 {{previous}} +S explicit_inst_class_template; int k2 = explicit_inst_class_template.n; -#endif #include "a.h" T::S implicit_inst_member_class_template; int k3 = implicit_inst_member_class_template.n; -#ifdef ERR2 -T::S explicit_inst_member_class_template; // expected-error {{must be imported from module 'tsv.e'}} -// expected-note@e.h:5 {{previous}} +T::S explicit_inst_member_class_template; int k4 = explicit_inst_member_class_template.n; -#endif T::E implicit_inst_member_enum_template; int k5 = decltype(implicit_inst_member_enum_template)::e; -#ifdef ERR3 -T::E explicit_inst_member_enum_template; // expected-error {{must be imported from module 'tsv.e'}} -// expected-note@e.h:5 {{previous}} +T::E explicit_inst_member_enum_template; int k6 = decltype(explicit_inst_member_enum_template)::e; -#endif - -#if ERR1 + ERR2 + ERR3 == 0 -// expected-no-diagnostics -#endif