Author: rsmith Date: Wed Apr 17 17:56:58 2019 New Revision: 358631 URL: http://llvm.org/viewvc/llvm-project?rev=358631&view=rev Log: [c++2a] Improve diagnostic for use of declaration from another TU's global module fragment.
We know that the declaration in question should have been introduced by a '#include', so try to figure out which one and suggest it. Don't suggest importing the global module fragment itself! Added: cfe/trunk/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Lex/PPDirectives.cpp cfe/trunk/lib/Sema/SemaLookup.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=358631&r1=358630&r2=358631&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Apr 17 17:56:58 2019 @@ -9238,6 +9238,12 @@ def err_module_unimported_use_header : E "%select{declaration|definition|default argument|" "explicit specialization|partial specialization}0 of %1 must be imported " "from module '%2' before it is required">; +def err_module_unimported_use_global_module_fragment : Error< + "%select{missing '#include'|missing '#include %3'}2; " + "%select{||default argument of |explicit specialization of |" + "partial specialization of }0%1 must be " + "%select{declared|defined|defined|declared|declared}0 " + "before it is used">; def err_module_unimported_use_multiple : Error< "%select{declaration|definition|default argument|" "explicit specialization|partial specialization}0 of %1 must be imported " Modified: cfe/trunk/lib/Lex/PPDirectives.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=358631&r1=358630&r2=358631&view=diff ============================================================================== --- cfe/trunk/lib/Lex/PPDirectives.cpp (original) +++ cfe/trunk/lib/Lex/PPDirectives.cpp Wed Apr 17 17:56:58 2019 @@ -614,9 +614,16 @@ Preprocessor::getModuleHeaderToIncludeFo SourceLocation Loc) { assert(M && "no module to include"); + // If the context is the global module fragment of some module, we never + // want to return that file; instead, we want the innermost include-guarded + // header that it included. + bool InGlobalModuleFragment = M->Kind == Module::GlobalModuleFragment; + // If we have a module import syntax, we shouldn't include a header to // make a particular module visible. - if (getLangOpts().ObjC) + if ((getLangOpts().ObjC || getLangOpts().CPlusPlusModules || + getLangOpts().ModulesTS) && + !InGlobalModuleFragment) return nullptr; Module *TopM = M->getTopLevelModule(); @@ -633,6 +640,13 @@ Preprocessor::getModuleHeaderToIncludeFo if (!FE) break; + if (InGlobalModuleFragment) { + if (getHeaderSearchInfo().isFileMultipleIncludeGuarded(FE)) + return FE; + Loc = SM.getIncludeLoc(ID); + continue; + } + bool InTextualHeader = false; for (auto Header : HeaderInfo.getModuleMap().findAllModulesForHeader(FE)) { if (!Header.getModule()->isSubModuleOf(TopM)) Modified: cfe/trunk/lib/Sema/SemaLookup.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=358631&r1=358630&r2=358631&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) +++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed Apr 17 17:56:58 2019 @@ -5073,7 +5073,7 @@ void Sema::diagnoseMissingImport(SourceL auto Merged = Context.getModulesWithMergedDefinition(Def); OwningModules.insert(OwningModules.end(), Merged.begin(), Merged.end()); - diagnoseMissingImport(Loc, Decl, Decl->getLocation(), OwningModules, MIK, + diagnoseMissingImport(Loc, Def, Def->getLocation(), OwningModules, MIK, Recover); } @@ -5093,12 +5093,58 @@ void Sema::diagnoseMissingImport(SourceL MissingImportKind MIK, bool Recover) { assert(!Modules.empty()); + auto NotePrevious = [&] { + 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; + case MissingImportKind::ExplicitSpecialization: + DiagID = diag::note_explicit_specialization_declared_here; + break; + case MissingImportKind::PartialSpecialization: + DiagID = diag::note_partial_specialization_declared_here; + break; + } + Diag(DeclLoc, DiagID); + }; + // Weed out duplicates from module list. llvm::SmallVector<Module*, 8> UniqueModules; llvm::SmallDenseSet<Module*, 8> UniqueModuleSet; - for (auto *M : Modules) + for (auto *M : Modules) { + if (M->Kind == Module::GlobalModuleFragment) + continue; if (UniqueModuleSet.insert(M).second) UniqueModules.push_back(M); + } + + if (UniqueModules.empty()) { + // All candidates were global module fragments. Try to suggest a #include. + const FileEntry *E = + PP.getModuleHeaderToIncludeForDiagnostics(UseLoc, Modules[0], DeclLoc); + // FIXME: Find a smart place to suggest inserting a #include, and add + // a FixItHint there. + Diag(UseLoc, diag::err_module_unimported_use_global_module_fragment) + << (int)MIK << Decl << !!E + << (E ? getIncludeStringForHeader(PP, E) : ""); + // Produce a "previous" note if it will point to a header rather than some + // random global module fragment. + // FIXME: Suppress the note backtrace even under + // -fdiagnostics-show-note-include-stack. + if (E) + NotePrevious(); + if (Recover) + createImplicitModuleImportForErrorRecovery(UseLoc, Modules[0]); + return; + } + Modules = UniqueModules; if (Modules.size() > 1) { @@ -5131,25 +5177,7 @@ void Sema::diagnoseMissingImport(SourceL << (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; - case MissingImportKind::ExplicitSpecialization: - DiagID = diag::note_explicit_specialization_declared_here; - break; - case MissingImportKind::PartialSpecialization: - DiagID = diag::note_partial_specialization_declared_here; - break; - } - Diag(DeclLoc, DiagID); + NotePrevious(); // Try to recover by implicitly importing this module. if (Recover) Added: cfe/trunk/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp?rev=358631&view=auto ============================================================================== --- cfe/trunk/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp (added) +++ cfe/trunk/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp Wed Apr 17 17:56:58 2019 @@ -0,0 +1,86 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: echo '#ifndef FOO_H' > %t/foo.h +// RUN: echo '#define FOO_H' >> %t/foo.h +// RUN: echo 'extern int in_header;' >> %t/foo.h +// RUN: echo '#endif' >> %t/foo.h +// RUN: %clang_cc1 -std=c++2a -I%t -emit-module-interface -DINTERFACE %s -o %t.pcm +// RUN: %clang_cc1 -std=c++2a -I%t -fmodule-file=%t.pcm -DIMPLEMENTATION %s -verify -fno-modules-error-recovery +// RUN: %clang_cc1 -std=c++2a -I%t -fmodule-file=%t.pcm %s -verify -fno-modules-error-recovery + +#ifdef INTERFACE +module; +#include "foo.h" +int global_module_fragment; +export module A; +export int exported; +int not_exported; +static int internal; + +module :private; +int not_exported_private; +static int internal_private; +#else + +#ifdef IMPLEMENTATION +module; +#endif + +void test_early() { + in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}} + // expected-note@*{{previous}} + + global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}} + + exported = 1; // expected-error {{must be imported from module 'A'}} + // expected-n...@p2.cpp:16 {{previous}} + + not_exported = 1; // expected-error {{undeclared identifier}} + + internal = 1; // expected-error {{undeclared identifier}} + + not_exported_private = 1; // expected-error {{undeclared identifier}} + + internal_private = 1; // expected-error {{undeclared identifier}} +} + +#ifdef IMPLEMENTATION +module A; +#else +import A; +#endif + +void test_late() { + in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}} + // expected-note@*{{previous}} + + global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}} + + exported = 1; + + not_exported = 1; +#ifndef IMPLEMENTATION + // expected-error@-2 {{undeclared identifier 'not_exported'; did you mean 'exported'}} + // expected-n...@p2.cpp:16 {{declared here}} +#endif + + internal = 1; +#ifndef IMPLEMENTATION + // FIXME: should not be visible here + // expected-error@-3 {{undeclared identifier}} +#endif + + not_exported_private = 1; +#ifndef IMPLEMENTATION + // FIXME: should not be visible here + // expected-error@-3 {{undeclared identifier}} +#endif + + internal_private = 1; +#ifndef IMPLEMENTATION + // FIXME: should not be visible here + // expected-error@-3 {{undeclared identifier}} +#endif +} + +#endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits