Author: Chuanqi Xu Date: 2023-11-10T10:58:07+08:00 New Revision: 0e7b30fa821dd4899227aa643030f4e4164f4e56
URL: https://github.com/llvm/llvm-project/commit/0e7b30fa821dd4899227aa643030f4e4164f4e56 DIFF: https://github.com/llvm/llvm-project/commit/0e7b30fa821dd4899227aa643030f4e4164f4e56.diff LOG: [C++20] [Modules] Enhance better diagnostic for implicit global module and module partitions Due to an oversight, when users use an unexported declaration from implicit global module, the diagnostic will show "please #include" instead of "please import". This patch corrects the behavior. Also previously, when users use an unexported declarations from module partitions, the diagnostic message will always show the partition name no matter if that partition name is visible to the users. Now the users may only see the partition name if the users are in the same module with the partition unit. Added: Modified: clang/lib/Sema/SemaLookup.cpp clang/test/CXX/module/module.import/p2.cpp clang/test/CXX/module/module.reach/ex1.cpp clang/test/CXX/module/module.reach/p2.cpp clang/test/Modules/export-language-linkage.cppm Removed: ################################################################################ diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp index be011b9f4368d2c..54b48f7ab80eb9c 100644 --- a/clang/lib/Sema/SemaLookup.cpp +++ b/clang/lib/Sema/SemaLookup.cpp @@ -5723,7 +5723,7 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl, llvm::SmallVector<Module*, 8> UniqueModules; llvm::SmallDenseSet<Module*, 8> UniqueModuleSet; for (auto *M : Modules) { - if (M->isGlobalModule() || M->isPrivateModule()) + if (M->isExplicitGlobalModule() || M->isPrivateModule()) continue; if (UniqueModuleSet.insert(M).second) UniqueModules.push_back(M); @@ -5755,6 +5755,28 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl, Modules = UniqueModules; + auto GetModuleNameForDiagnostic = [this](const Module *M) -> std::string { + if (M->isModuleMapModule()) + return M->getFullModuleName(); + + Module *CurrentModule = getCurrentModule(); + + if (M->isImplicitGlobalModule()) + M = M->getTopLevelModule(); + + bool IsInTheSameModule = + CurrentModule && CurrentModule->getPrimaryModuleInterfaceName() == + M->getPrimaryModuleInterfaceName(); + + // If the current module unit is in the same module with M, it is OK to show + // the partition name. Otherwise, it'll be sufficient to show the primary + // module name. + if (IsInTheSameModule) + return M->getTopLevelModuleName().str(); + else + return M->getPrimaryModuleInterfaceName().str(); + }; + if (Modules.size() > 1) { std::string ModuleList; unsigned N = 0; @@ -5764,7 +5786,7 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl, ModuleList += "[...]"; break; } - ModuleList += M->getFullModuleName(); + ModuleList += GetModuleNameForDiagnostic(M); } Diag(UseLoc, diag::err_module_unimported_use_multiple) @@ -5772,7 +5794,7 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl, } else { // FIXME: Add a FixItHint that imports the corresponding module. Diag(UseLoc, diag::err_module_unimported_use) - << (int)MIK << Decl << Modules[0]->getFullModuleName(); + << (int)MIK << Decl << GetModuleNameForDiagnostic(Modules[0]); } NotePrevious(); diff --git a/clang/test/CXX/module/module.import/p2.cpp b/clang/test/CXX/module/module.import/p2.cpp index 0c02d253f3a898f..ef6006811e77631 100644 --- a/clang/test/CXX/module/module.import/p2.cpp +++ b/clang/test/CXX/module/module.import/p2.cpp @@ -23,8 +23,8 @@ export A f(); //--- Use.cpp import M; void test() { - A a; // expected-error {{declaration of 'A' must be imported from module 'M:impl'}} - // expected-error@-1 {{definition of 'A' must be imported from module 'M:impl'}} expected-error@-1 {{}} + A a; // expected-error {{definition of 'A' must be imported from module 'M' before it is required}} + // expected-error@-1 {{definition of 'A' must be imported from module 'M' before it is required}} expected-error@-1 {{}} // expected-n...@impl.cppm:2 {{declaration here is not visible}} // expected-n...@impl.cppm:2 {{definition here is not reachable}} expected-n...@impl.cppm:2 {{}} } @@ -41,8 +41,8 @@ void test() { export module B; import M; void test() { - A a; // expected-error {{declaration of 'A' must be imported from module 'M:impl'}} - // expected-error@-1 {{definition of 'A' must be imported from module 'M:impl'}} expected-error@-1 {{}} + A a; // expected-error {{declaration of 'A' must be imported from module 'M'}} + // expected-error@-1 {{definition of 'A' must be imported from module 'M'}} expected-error@-1 {{}} // expected-n...@impl.cppm:2 {{declaration here is not visible}} // expected-n...@impl.cppm:2 {{definition here is not reachable}} expected-n...@impl.cppm:2 {{}} } diff --git a/clang/test/CXX/module/module.reach/ex1.cpp b/clang/test/CXX/module/module.reach/ex1.cpp index a1e38c4c88a2885..96d06f9a3e0f69a 100644 --- a/clang/test/CXX/module/module.reach/ex1.cpp +++ b/clang/test/CXX/module/module.reach/ex1.cpp @@ -37,10 +37,10 @@ export void f(B b = B()); //--- X.cppm export module X; import M; -B b3; // expected-error {{definition of 'B' must be imported from module 'M:B' before it is required}} expected-error {{}} +B b3; // expected-error {{definition of 'B' must be imported from module 'M' before it is required}} expected-error {{}} // expected-note@* {{definition here is not reachable}} expected-note@* {{}} // FIXME: We should emit an error for unreachable definition of B. void g() { f(); } -void g1() { f(B()); } // expected-error 1+{{definition of 'B' must be imported from module 'M:B' before it is required}} +void g1() { f(B()); } // expected-error 1+{{definition of 'B' must be imported from module 'M' before it is required}} // expected-note@* 1+{{definition here is not reachable}} // expected-n...@m.cppm:5 {{passing argument to parameter 'b' here}} diff --git a/clang/test/CXX/module/module.reach/p2.cpp b/clang/test/CXX/module/module.reach/p2.cpp index 6ccf6ee6d0566d3..9242e6f2457e8f1 100644 --- a/clang/test/CXX/module/module.reach/p2.cpp +++ b/clang/test/CXX/module/module.reach/p2.cpp @@ -17,6 +17,6 @@ export A f(); //--- UseStrict.cpp import M; void test() { - auto a = f(); // expected-error {{definition of 'A' must be imported from module 'M:impl' before it is required}} expected-error{{}} + auto a = f(); // expected-error {{definition of 'A' must be imported from module 'M' before it is required}} expected-error{{}} // expected-note@* {{definition here is not reachable}} expected-note@* {{}} } diff --git a/clang/test/Modules/export-language-linkage.cppm b/clang/test/Modules/export-language-linkage.cppm index 420e4be0ce45b63..462b28d36cb44b5 100644 --- a/clang/test/Modules/export-language-linkage.cppm +++ b/clang/test/Modules/export-language-linkage.cppm @@ -37,7 +37,7 @@ int use() { f1(); f2(); f3(); - unexported(); // expected-error {{missing '#include'; 'unexported' must be declared before it is used}} + unexported(); // expected-error {{declaration of 'unexported' must be imported from module 'a' before it is required}} // expected-n...@a.cppm:15 {{declaration here is not visible}} return foo(); } @@ -58,6 +58,6 @@ int use() { } int use_of_nonexported() { - return h(); // expected-error {{missing '#include'; 'h' must be declared before it is used}} + return h(); // expected-error {{declaration of 'h' must be imported from module 'c' before it is required}} // expected-n...@c.cppm:4 {{declaration here is not visible}} } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
