[clang] 0e7b30f - [C++20] [Modules] Enhance better diagnostic for implicit global module and module partitions

Chuanqi Xu via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 9 18:58:41 PST 2023


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 at -1 {{definition of 'A' must be imported from module 'M:impl'}} expected-error at -1 {{}}
+  A a; // expected-error {{definition of 'A' must be imported from module 'M' before it is required}}
+       // expected-error at -1 {{definition of 'A' must be imported from module 'M' before it is required}} expected-error at -1 {{}}
        // expected-note at impl.cppm:2 {{declaration here is not visible}}
        // expected-note at impl.cppm:2 {{definition here is not reachable}} expected-note at 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 at -1 {{definition of 'A' must be imported from module 'M:impl'}} expected-error at -1 {{}}
+  A a; // expected-error {{declaration of 'A' must be imported from module 'M'}}
+       // expected-error at -1 {{definition of 'A' must be imported from module 'M'}} expected-error at -1 {{}}
        // expected-note at impl.cppm:2 {{declaration here is not visible}}
        // expected-note at impl.cppm:2 {{definition here is not reachable}} expected-note at 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-note at 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-note at 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-note at c.cppm:4 {{declaration here is not visible}}
 }


        


More information about the cfe-commits mailing list