diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index d318a20928203d7b457812e1638d201d10ed16ae..71cba0183b80528179687fc5b8399bef09dd3079 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -7637,9 +7637,11 @@ 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< - "declaration of %0 must be imported from module '%1' before it is required">; -def err_module_private_definition : Error< - "definition of %0 must be imported from module '%1' before it is required">; + "%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_import_in_extern_c : Error< "import of C++ module '%0' appears within extern \"C\" language linkage " "specification">; diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 685a1ab2435e872df32f32e73e60e81026d0ee3b..691f21d43359c10fca53543d9f88f8b3d0fed029 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1730,6 +1730,11 @@ public: void createImplicitModuleImportForErrorRecovery(SourceLocation Loc, Module *Mod); + /// \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); + /// \brief Retrieve a suitable printing policy. PrintingPolicy getPrintingPolicy() const { return getPrintingPolicy(Context, PP); diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 202f027f6f80884e8502274a253d30fdb1693517..b5ef3a42d17f2ab83cbb234d326a8e14a687fc5e 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -4661,6 +4661,50 @@ static NamedDecl *getDefinitionToImport(NamedDecl *D) { return nullptr; } +void Sema::diagnoseMissingImport(SourceLocation Loc, NamedDecl *Decl, + bool NeedDefinition, bool Recover) { + assert(!isVisible(Decl) && "missing import for non-hidden decl?"); + + // Suggest importing a module providing the definition of this entity, if + // possible. + NamedDecl *Def = getDefinitionToImport(Decl); + if (!Def) + Def = Decl; + + // FIXME: Add a Fix-It that imports the corresponding module or includes + // the header. + Module *Owner = getOwningModule(Decl); + assert(Owner && "definition of hidden declaration is not in a module"); + + auto Merged = Context.getModulesWithMergedDefinition(Decl); + if (!Merged.empty()) { + std::string ModuleList; + ModuleList += "\n "; + ModuleList += Owner->getFullModuleName(); + unsigned N = 0; + for (Module *M : Merged) { + ModuleList += "\n "; + if (++N == 5 && Merged.size() != N) { + ModuleList += "[...]"; + break; + } + ModuleList += M->getFullModuleName(); + } + + Diag(Loc, diag::err_module_private_declaration_multiple) + << NeedDefinition << Decl << ModuleList; + } else { + Diag(Loc, diag::err_module_private_declaration) + << NeedDefinition << Decl << Owner->getFullModuleName(); + } + Diag(Decl->getLocation(), NeedDefinition ? diag::note_previous_definition + : diag::note_previous_declaration); + + // Try to recover by implicitly importing this module. + if (Recover) + createImplicitModuleImportForErrorRecovery(Loc, Owner); +} + /// \brief Diagnose a successfully-corrected typo. Separated from the correction /// itself to allow external validation of the result, etc. /// @@ -4687,23 +4731,8 @@ void Sema::diagnoseTypo(const TypoCorrection &Correction, NamedDecl *Decl = Correction.getCorrectionDecl(); assert(Decl && "import required but no declaration to import"); - // Suggest importing a module providing the definition of this entity, if - // possible. - NamedDecl *Def = getDefinitionToImport(Decl); - if (!Def) - Def = Decl; - Module *Owner = getOwningModule(Def); - assert(Owner && "definition of hidden declaration is not in a module"); - - Diag(Correction.getCorrectionRange().getBegin(), - diag::err_module_private_declaration) - << Def << Owner->getFullModuleName(); - Diag(Def->getLocation(), diag::note_previous_declaration); - - // Recover by implicitly importing this module. - if (ErrorRecovery) - createImplicitModuleImportForErrorRecovery( - Correction.getCorrectionRange().getBegin(), Owner); + diagnoseMissingImport(Correction.getCorrectionRange().getBegin(), Decl, + /*NeedDefinition*/ false, ErrorRecovery); return; } diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index 5734f3b2ed52485455770fef2ad69f63e22f8977..d3787ec44b12da4b1a59a66559804076e88aaf91 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -5239,20 +5239,8 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T, // If we know about the definition but it is not visible, complain. NamedDecl *SuggestedDef = nullptr; if (!Diagnoser.Suppressed && Def && - !hasVisibleDefinition(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 = getOwningModule(SuggestedDef); - Diag(Loc, diag::err_module_private_definition) - << T << Owner->getFullModuleName(); - Diag(SuggestedDef->getLocation(), diag::note_previous_definition); - - // Try to recover by implicitly importing this module. - createImplicitModuleImportForErrorRecovery(Loc, Owner); - } + !hasVisibleDefinition(Def, &SuggestedDef)) + diagnoseMissingImport(Loc, SuggestedDef, /*NeedDefinition*/true); // We lock in the inheritance model once somebody has asked us to ensure // that a pointer-to-member type is complete. diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 7e1bd07f1ee385d0604481a6d7b2eb8317cf195b..5bb0bec4f5690b1200fb92588ac133812aa6818e 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -5764,8 +5764,5 @@ void ASTWriter::DeclarationMarkedOpenMPThreadPrivate(const Decl *D) { void ASTWriter::RedefinedHiddenDefinition(const NamedDecl *D, Module *M) { assert(!WritingAST && "Already writing the AST!"); assert(D->isHidden() && "expected a hidden declaration"); - if (!D->isFromASTFile()) - return; - DeclUpdates[D].push_back(DeclUpdate(UPD_DECL_EXPORTED, M)); } diff --git a/test/Modules/Inputs/submodules-merge-defs/defs.h b/test/Modules/Inputs/submodules-merge-defs/defs.h index ad3711f073f95d9654344f08035ad6eb30fff7c3..b98cbc3a7641eb7a51e2344d0aec36f8406b19db 100644 --- a/test/Modules/Inputs/submodules-merge-defs/defs.h +++ b/test/Modules/Inputs/submodules-merge-defs/defs.h @@ -31,7 +31,7 @@ template<typename T> struct F { template<typename T> int F<T>::f() { return 0; } template<typename T> template<typename U> int F<T>::g() { return 0; } template<typename T> int F<T>::n = 0; -template<> template<typename U> int F<char>::g() { return 0; } +//template<> template<typename U> int F<char>::g() { return 0; } // FIXME: Re-enable this once we support merging member specializations. template<> struct F<void> { int h(); }; inline int F<void>::h() { return 0; } template<typename T> struct F<T *> { int i(); }; diff --git a/test/Modules/Inputs/submodules-merge-defs/module.modulemap b/test/Modules/Inputs/submodules-merge-defs/module.modulemap index f8ae60fe44fc72a51b6f4a1f74f78f86e1cdb750..3b5493e2b8bb02718e1937ade39ca14d61cfe14d 100644 --- a/test/Modules/Inputs/submodules-merge-defs/module.modulemap +++ b/test/Modules/Inputs/submodules-merge-defs/module.modulemap @@ -2,6 +2,7 @@ module "stuff" { textual header "defs.h" module "empty" { header "empty.h" } module "use" { header "use-defs.h" } + module "use-2" { requires use_defs_twice header "use-defs-2.h" } } module "redef" { diff --git a/test/Modules/Inputs/submodules-merge-defs/use-defs-2.h b/test/Modules/Inputs/submodules-merge-defs/use-defs-2.h new file mode 100644 index 0000000000000000000000000000000000000000..31c69c6a447d8f32c817f83383c8f57834c6f833 --- /dev/null +++ b/test/Modules/Inputs/submodules-merge-defs/use-defs-2.h @@ -0,0 +1 @@ +#include "defs.h" diff --git a/test/Modules/module-private.cpp b/test/Modules/module-private.cpp index 9213a0f20cbcad1d4ca4e41a55ae667add3bb13f..478d36dd4d59921adafcfe92eed4da855fd42fde 100644 --- a/test/Modules/module-private.cpp +++ b/test/Modules/module-private.cpp @@ -14,7 +14,7 @@ void test() { int test_broken() { HiddenStruct hidden; // \ // expected-error{{must use 'struct' tag to refer to type 'HiddenStruct' in this scope}} \ - // expected-error{{definition of 'struct HiddenStruct' must be imported}} + // expected-error{{definition of 'HiddenStruct' must be imported}} // expected-note@Inputs/module_private_left.h:3 {{previous definition is here}} Integer i; // expected-error{{unknown type name 'Integer'}} diff --git a/test/Modules/submodules-merge-defs.cpp b/test/Modules/submodules-merge-defs.cpp index e942335151e47cf1ce8137501b83c8a8f79ddaaa..52b12ef850afe355fd0646fd57047d024752f2dd 100644 --- a/test/Modules/submodules-merge-defs.cpp +++ b/test/Modules/submodules-merge-defs.cpp @@ -4,6 +4,7 @@ // RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -fmodules-local-submodule-visibility -DTEXTUAL // RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -fmodules-local-submodule-visibility // RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodule-maps -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -fmodules-local-submodule-visibility -DTEXTUAL -DEARLY_INDIRECT_INCLUDE +// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules-cache-path=%t -fmodules -I %S/Inputs/submodules-merge-defs %s -verify -fno-modules-error-recovery -fmodules-local-submodule-visibility -fmodule-feature use_defs_twice -DIMPORT_USE_2 // Trigger import of definitions, but don't make them visible. #include "empty.h" @@ -11,7 +12,14 @@ #include "indirect.h" #endif -A pre_a; // expected-error {{must be imported}} expected-error {{must use 'struct'}} +A pre_a; // expected-error {{must use 'struct'}} +#ifdef IMPORT_USE_2 +// expected-error-re@-2 {{must be imported from one of {{.*}}stuff.use{{.*}}stuff.use-2}} +#elif EARLY_INDIRECT_INCLUDE +// expected-error@-4 {{must be imported from module 'merged-defs'}} +#else +// expected-error@-6 {{must be imported from module 'stuff.use'}} +#endif // expected-note@defs.h:1 +{{here}} // expected-note@defs.h:2 +{{here}} int pre_use_a = use_a(pre_a); // expected-error {{'A' must be imported}} expected-error {{'use_a' must be imported}} @@ -46,6 +54,8 @@ J<> pre_j; // expected-error {{must be imported}} expected-error {{too few}} // Make definitions from second module visible. #ifdef TEXTUAL #include "import-and-redefine.h" +#elif defined IMPORT_USE_2 +#include "use-defs-2.h" #else #include "merged-defs.h" #endif @@ -61,13 +71,6 @@ int post_use_dx = use_dx(post_dx); int post_e = E(0); int post_ff = F<char>().f(); int post_fg = F<char>().g<int>(); -#ifdef EARLY_INDIRECT_INCLUDE -// FIXME: Properly track the owning module for a member specialization. -// expected-error@defs.h:34 {{redefinition}} -// expected-note@defs.h:34 {{previous definition}} -// expected-error@-5 {{no matching member function}} -// expected-note@defs.h:34 {{substitution failure}} -#endif J<> post_j; template<typename T, int N, template<typename> class K> struct J; J<> post_j2;