r208150 - If an instantiation of a template is required to be a complete type, check
Richard Smith
richard-llvm at metafoo.co.uk
Tue May 6 19:25:44 PDT 2014
Author: rsmith
Date: Tue May 6 21:25:43 2014
New Revision: 208150
URL: http://llvm.org/viewvc/llvm-project?rev=208150&view=rev
Log:
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.
Modified:
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaType.cpp
cfe/trunk/test/Modules/Inputs/cxx-templates-b-impl.h
cfe/trunk/test/Modules/Inputs/cxx-templates-common.h
cfe/trunk/test/Modules/cxx-templates.cpp
cfe/trunk/test/Modules/template-specialization-visibility.cpp
Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Tue May 6 21:25:43 2014
@@ -98,6 +98,7 @@ LANGOPT(Modules , 1, 0, "modul
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__)")
Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Tue May 6 21:25:43 2014
@@ -314,6 +314,8 @@ def ast_dump_lookups : Flag<["-"], "ast-
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 {
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue May 6 21:25:43 2014
@@ -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 {
Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue May 6 21:25:43 2014
@@ -1359,6 +1359,7 @@ static void ParseLangArgs(LangOptions &O
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);
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue May 6 21:25:43 2014
@@ -13177,7 +13177,12 @@ void Sema::ActOnModuleInclude(SourceLoca
/*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,
Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Tue May 6 21:25:43 2014
@@ -4577,9 +4577,9 @@ void Sema::diagnoseTypo(const TypoCorrec
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;
}
Modified: cfe/trunk/lib/Sema/SemaType.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaType.cpp (original)
+++ cfe/trunk/lib/Sema/SemaType.cpp Tue May 6 21:25:43 2014
@@ -5076,6 +5076,69 @@ bool Sema::RequireCompleteType(SourceLoc
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<CXXRecordDecl>(D)) {
+ if (auto *TD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
+ auto From = TD->getInstantiatedFrom();
+ if (auto *CTD = From.dyn_cast<ClassTemplateDecl*>()) {
+ 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<EnumDecl>(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(Sourc
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
Modified: cfe/trunk/test/Modules/Inputs/cxx-templates-b-impl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-templates-b-impl.h?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-templates-b-impl.h (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-templates-b-impl.h Tue May 6 21:25:43 2014
@@ -3,3 +3,10 @@ struct DefinedInBImpl {
struct Inner {};
friend void FoundByADL(DefinedInBImpl);
};
+
+ at import cxx_templates_common;
+template struct TemplateInstantiationVisibility<char[1]>;
+extern template struct TemplateInstantiationVisibility<char[2]>;
+template<> struct TemplateInstantiationVisibility<char[3]> {};
+extern TemplateInstantiationVisibility<char[4]>::type
+ TemplateInstantiationVisibility_ImplicitInstantiation;
Modified: cfe/trunk/test/Modules/Inputs/cxx-templates-common.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/cxx-templates-common.h?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/cxx-templates-common.h (original)
+++ cfe/trunk/test/Modules/Inputs/cxx-templates-common.h Tue May 6 21:25:43 2014
@@ -21,3 +21,5 @@ namespace Std {
extern T g();
}
}
+
+template<typename T> struct TemplateInstantiationVisibility { typedef int type; };
Modified: cfe/trunk/test/Modules/cxx-templates.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/cxx-templates.cpp?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/test/Modules/cxx-templates.cpp (original)
+++ cfe/trunk/test/Modules/cxx-templates.cpp Tue May 6 21:25:43 2014
@@ -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 at Inputs/cxx-templates-a.h:19 {{definition of 'DefinedInBImpl' must be imported}}
- // expected-note at Inputs/cxx-templates-b-impl.h:1 {{definition is here}}
+ // expected-note at Inputs/cxx-templates-b-impl.h:1 +{{definition is here}}
+ // expected-error at Inputs/cxx-templates-a.h:19 +{{}}
+ // expected-error at 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<int> cti;
CommonTemplate<int>::E eee = CommonTemplate<int>::c;
+
+ TemplateInstantiationVisibility<char[1]> tiv1;
+ TemplateInstantiationVisibility<char[2]> tiv2;
+ TemplateInstantiationVisibility<char[3]> tiv3; // expected-error {{must be imported from module 'cxx_templates_b_impl'}}
+ // expected-note at cxx-templates-b-impl.h:10 {{previous definition is here}}
+ TemplateInstantiationVisibility<char[4]> tiv4;
}
RedeclaredAsFriend<int> raf1;
Modified: cfe/trunk/test/Modules/template-specialization-visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/template-specialization-visibility.cpp?rev=208150&r1=208149&r2=208150&view=diff
==============================================================================
--- cfe/trunk/test/Modules/template-specialization-visibility.cpp (original)
+++ cfe/trunk/test/Modules/template-specialization-visibility.cpp Tue May 6 21:25:43 2014
@@ -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<int> implicit_inst_class_template;
int k1 = implicit_inst_class_template.n;
-#ifdef ERR1
-S<char> explicit_inst_class_template; // expected-error {{must be imported from module 'tsv.e'}}
-// expected-note at e.h:4 {{previous}}
+S<char> explicit_inst_class_template;
int k2 = explicit_inst_class_template.n;
-#endif
#include "a.h"
T<int>::S implicit_inst_member_class_template;
int k3 = implicit_inst_member_class_template.n;
-#ifdef ERR2
-T<char>::S explicit_inst_member_class_template; // expected-error {{must be imported from module 'tsv.e'}}
-// expected-note at e.h:5 {{previous}}
+T<char>::S explicit_inst_member_class_template;
int k4 = explicit_inst_member_class_template.n;
-#endif
T<int>::E implicit_inst_member_enum_template;
int k5 = decltype(implicit_inst_member_enum_template)::e;
-#ifdef ERR3
-T<char>::E explicit_inst_member_enum_template; // expected-error {{must be imported from module 'tsv.e'}}
-// expected-note at e.h:5 {{previous}}
+T<char>::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
More information about the cfe-commits
mailing list