Skip to content

Commit 0e7b30f

Browse files
committed
[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.
1 parent dc3faf0 commit 0e7b30f

File tree

5 files changed

+34
-12
lines changed

5 files changed

+34
-12
lines changed

clang/lib/Sema/SemaLookup.cpp

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5723,7 +5723,7 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl,
57235723
llvm::SmallVector<Module*, 8> UniqueModules;
57245724
llvm::SmallDenseSet<Module*, 8> UniqueModuleSet;
57255725
for (auto *M : Modules) {
5726-
if (M->isGlobalModule() || M->isPrivateModule())
5726+
if (M->isExplicitGlobalModule() || M->isPrivateModule())
57275727
continue;
57285728
if (UniqueModuleSet.insert(M).second)
57295729
UniqueModules.push_back(M);
@@ -5755,6 +5755,28 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl,
57555755

57565756
Modules = UniqueModules;
57575757

5758+
auto GetModuleNameForDiagnostic = [this](const Module *M) -> std::string {
5759+
if (M->isModuleMapModule())
5760+
return M->getFullModuleName();
5761+
5762+
Module *CurrentModule = getCurrentModule();
5763+
5764+
if (M->isImplicitGlobalModule())
5765+
M = M->getTopLevelModule();
5766+
5767+
bool IsInTheSameModule =
5768+
CurrentModule && CurrentModule->getPrimaryModuleInterfaceName() ==
5769+
M->getPrimaryModuleInterfaceName();
5770+
5771+
// If the current module unit is in the same module with M, it is OK to show
5772+
// the partition name. Otherwise, it'll be sufficient to show the primary
5773+
// module name.
5774+
if (IsInTheSameModule)
5775+
return M->getTopLevelModuleName().str();
5776+
else
5777+
return M->getPrimaryModuleInterfaceName().str();
5778+
};
5779+
57585780
if (Modules.size() > 1) {
57595781
std::string ModuleList;
57605782
unsigned N = 0;
@@ -5764,15 +5786,15 @@ void Sema::diagnoseMissingImport(SourceLocation UseLoc, const NamedDecl *Decl,
57645786
ModuleList += "[...]";
57655787
break;
57665788
}
5767-
ModuleList += M->getFullModuleName();
5789+
ModuleList += GetModuleNameForDiagnostic(M);
57685790
}
57695791

57705792
Diag(UseLoc, diag::err_module_unimported_use_multiple)
57715793
<< (int)MIK << Decl << ModuleList;
57725794
} else {
57735795
// FIXME: Add a FixItHint that imports the corresponding module.
57745796
Diag(UseLoc, diag::err_module_unimported_use)
5775-
<< (int)MIK << Decl << Modules[0]->getFullModuleName();
5797+
<< (int)MIK << Decl << GetModuleNameForDiagnostic(Modules[0]);
57765798
}
57775799

57785800
NotePrevious();

clang/test/CXX/module/module.import/p2.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ export A f();
2323
//--- Use.cpp
2424
import M;
2525
void test() {
26-
A a; // expected-error {{declaration of 'A' must be imported from module 'M:impl'}}
27-
// expected-error@-1 {{definition of 'A' must be imported from module 'M:impl'}} expected-error@-1 {{}}
26+
A a; // expected-error {{definition of 'A' must be imported from module 'M' before it is required}}
27+
// expected-error@-1 {{definition of 'A' must be imported from module 'M' before it is required}} expected-error@-1 {{}}
2828
// [email protected]:2 {{declaration here is not visible}}
2929
// [email protected]:2 {{definition here is not reachable}} [email protected]:2 {{}}
3030
}
@@ -41,8 +41,8 @@ void test() {
4141
export module B;
4242
import M;
4343
void test() {
44-
A a; // expected-error {{declaration of 'A' must be imported from module 'M:impl'}}
45-
// expected-error@-1 {{definition of 'A' must be imported from module 'M:impl'}} expected-error@-1 {{}}
44+
A a; // expected-error {{declaration of 'A' must be imported from module 'M'}}
45+
// expected-error@-1 {{definition of 'A' must be imported from module 'M'}} expected-error@-1 {{}}
4646
// [email protected]:2 {{declaration here is not visible}}
4747
// [email protected]:2 {{definition here is not reachable}} [email protected]:2 {{}}
4848
}

clang/test/CXX/module/module.reach/ex1.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ export void f(B b = B());
3737
//--- X.cppm
3838
export module X;
3939
import M;
40-
B b3; // expected-error {{definition of 'B' must be imported from module 'M:B' before it is required}} expected-error {{}}
40+
B b3; // expected-error {{definition of 'B' must be imported from module 'M' before it is required}} expected-error {{}}
4141
// expected-note@* {{definition here is not reachable}} expected-note@* {{}}
4242
// FIXME: We should emit an error for unreachable definition of B.
4343
void g() { f(); }
44-
void g1() { f(B()); } // expected-error 1+{{definition of 'B' must be imported from module 'M:B' before it is required}}
44+
void g1() { f(B()); } // expected-error 1+{{definition of 'B' must be imported from module 'M' before it is required}}
4545
// expected-note@* 1+{{definition here is not reachable}}
4646
// [email protected]:5 {{passing argument to parameter 'b' here}}

clang/test/CXX/module/module.reach/p2.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ export A f();
1717
//--- UseStrict.cpp
1818
import M;
1919
void test() {
20-
auto a = f(); // expected-error {{definition of 'A' must be imported from module 'M:impl' before it is required}} expected-error{{}}
20+
auto a = f(); // expected-error {{definition of 'A' must be imported from module 'M' before it is required}} expected-error{{}}
2121
// expected-note@* {{definition here is not reachable}} expected-note@* {{}}
2222
}

clang/test/Modules/export-language-linkage.cppm

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ int use() {
3737
f1();
3838
f2();
3939
f3();
40-
unexported(); // expected-error {{missing '#include'; 'unexported' must be declared before it is used}}
40+
unexported(); // expected-error {{declaration of 'unexported' must be imported from module 'a' before it is required}}
4141
// [email protected]:15 {{declaration here is not visible}}
4242
return foo();
4343
}
@@ -58,6 +58,6 @@ int use() {
5858
}
5959

6060
int use_of_nonexported() {
61-
return h(); // expected-error {{missing '#include'; 'h' must be declared before it is used}}
61+
return h(); // expected-error {{declaration of 'h' must be imported from module 'c' before it is required}}
6262
// [email protected]:4 {{declaration here is not visible}}
6363
}

0 commit comments

Comments
 (0)