iains created this revision.
Herald added a subscriber: ChuanqiXu.
Herald added a project: All.
iains added a reviewer: ChuanqiXu.
iains added a comment.
iains published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
notes:
a) I need to update the commit log message so that it formats properly in the
browser
b) since we are now seeing performance complaints against modules, and some of
these routines are used many times (sometimes several time per name), I have
tried to structure tests so that the cheapest ones execute first - and likewise
the utility routines.
Some of the testcases committed during the merging of modules contain
FIXMEs concerning the visibility of declarations with internal-linkage in
module interface units as seen from module implementation units.
This is, in part, caused by an oversight in the utility routine that determines
whether a module is part of the current TU or not.
The lookup definition for `visible` was also too wide, failing to account
cases where a module was visible, but some declarations from it were not.
This patch has three components:
1/ Fixing the behaviour of the `isModuleUnitOfCurrentTU` utility function to
return the correct result for module implementation units.
2/ Checking for visibility of imported decls and excluding thsose with
internal linkage (when from a different TU) or from a private module
fragment.
3/ Updating the diagnostics in reponse to 1 and 2, in particular dealing
better with the case that the user attempts to use a module-local linkage
item in an importing TU (we now provide a fixit to indicate that the item
needs to be exported to be used). When we search for possible "typos",
which we do in response to failing to find an entity, we do so allowing
finds of 'hidden' names. Before we offer names found this as possible
typo fixes, we need to check that they would be viable if not hidden.
There are a number of basically mechanical changes to tests to accommodate
the fixed rejection of relevant declarations and the improved diagnostic
messages.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D145965
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Lookup.h
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaModule.cpp
clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
clang/test/CXX/module/basic/basic.def.odr/p4.cppm
clang/test/CXX/module/basic/basic.link/p2.cppm
clang/test/CXX/module/module.import/p2.cpp
clang/test/CXX/module/module.interface/p2.cpp
clang/test/CXX/module/module.interface/p7.cpp
clang/test/CXX/module/module.reach/ex1.cpp
clang/test/CXX/module/module.reach/p2.cpp
clang/test/CXX/module/module.reach/p5.cpp
clang/test/Modules/Reachability-template-default-arg.cpp
clang/test/Modules/cxx20-10-1-ex2.cpp
clang/test/Modules/deduction-guide3.cppm
clang/test/Modules/diagnose-missing-import.m
Index: clang/test/Modules/diagnose-missing-import.m
===================================================================
--- clang/test/Modules/diagnose-missing-import.m
+++ clang/test/Modules/diagnose-missing-import.m
@@ -6,9 +6,9 @@
void foo(void) {
XYZLogEvent(xyzRiskyCloseOpenParam, xyzRiskyCloseOpenParam); // expected-error {{call to undeclared function 'XYZLogEvent'; ISO C99 and later do not support implicit function declarations}} \
- expected-error {{declaration of 'XYZLogEvent' must be imported}} \
- expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}} \
- expected-error {{declaration of 'xyzRiskyCloseOpenParam' must be imported from module 'NCI.A'}}
+ expected-error {{declaration of 'XYZLogEvent' is internal to 'NCI.A'}} \
+ expected-error {{declaration of 'xyzRiskyCloseOpenParam' is internal to 'NCI.A'}} \
+ expected-error {{declaration of 'xyzRiskyCloseOpenParam' is internal to 'NCI.A'}}
}
// expected-note@Inputs/diagnose-missing-import/a.h:5 {{declaration here is not visible}}
Index: clang/test/Modules/deduction-guide3.cppm
===================================================================
--- clang/test/Modules/deduction-guide3.cppm
+++ clang/test/Modules/deduction-guide3.cppm
@@ -19,8 +19,8 @@
//--- Use.cpp
import Templ;
void func() {
- Templ t(5); // expected-error {{declaration of 'Templ' must be imported from module 'Templ' before it is required}}
+ Templ t(5); // expected-error {{declaration of 'Templ' is private to module 'Templ'}}
// expected-error@-1 {{unknown type name 'Templ'}}
- // expected-n...@templ.cppm:3 {{declaration here is not visible}}
+ // expected-n...@templ.cppm:3 {{export the declaration to make it available}}
}
Index: clang/test/Modules/cxx20-10-1-ex2.cpp
===================================================================
--- clang/test/Modules/cxx20-10-1-ex2.cpp
+++ clang/test/Modules/cxx20-10-1-ex2.cpp
@@ -53,8 +53,8 @@
//--- std10-1-ex2-tu6.cpp
import B;
// error, n is module-local and this is not a module.
-int &c = n; // expected-error {{declaration of 'n' must be imported}}
- // expected-note@* {{declaration here is not visible}}
+int &c = n; // expected-error {{declaration of 'n' is private to module 'B'}}
+ // expected-note@* {{export the declaration to make it available}}
//--- std10-1-ex2-tu7.cpp
// expected-no-diagnostics
Index: clang/test/Modules/Reachability-template-default-arg.cpp
===================================================================
--- clang/test/Modules/Reachability-template-default-arg.cpp
+++ clang/test/Modules/Reachability-template-default-arg.cpp
@@ -18,6 +18,6 @@
import template_default_arg;
void bar() {
A<> a0;
- A<t> a1; // expected-error {{declaration of 't' must be imported from module 'template_default_arg' before it is required}}
- // expected-note@* {{declaration here is not visible}}
+ A<t> a1; // expected-error {{declaration of 't' is private to module 'template_default_arg'}}
+ // expected-note@* {{export the declaration to make it available}}
}
Index: clang/test/CXX/module/module.reach/p5.cpp
===================================================================
--- clang/test/CXX/module/module.reach/p5.cpp
+++ clang/test/CXX/module/module.reach/p5.cpp
@@ -14,5 +14,5 @@
export module B;
import A;
Y y; // OK, definition of X is reachable
-X x; // expected-error {{declaration of 'X' must be imported from module 'A' before it is required}}
- // expected-note@* {{declaration here is not visible}}
+X x; // expected-error {{declaration of 'X' is private to module 'A'}}
+ // expected-note@* {{export the declaration to make it available}}
Index: clang/test/CXX/module/module.reach/p2.cpp
===================================================================
--- clang/test/CXX/module/module.reach/p2.cpp
+++ clang/test/CXX/module/module.reach/p2.cpp
@@ -17,6 +17,6 @@
//--- 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{{}}
- // expected-note@* {{definition here is not reachable}} expected-note@* {{}}
+ auto a = f(); // expected-error 1+{{definition of 'A' is private to module 'M:impl'}}
+ // expected-note@* 1+{{export the definition to make it available}}
}
Index: clang/test/CXX/module/module.reach/ex1.cpp
===================================================================
--- clang/test/CXX/module/module.reach/ex1.cpp
+++ clang/test/CXX/module/module.reach/ex1.cpp
@@ -37,10 +37,10 @@
//--- 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 {{}}
- // expected-note@* {{definition here is not reachable}} expected-note@* {{}}
-// FIXME: We should emit an error for unreachable definition of B.
+B b3; // expected-error {{definition of 'B' is private to module 'M:B'}} expected-error {{}}
+ // expected-note@* {{export the definition to make it available}} expected-note@* {{}}
+
void g() { f(); }
-void g1() { f(B()); } // expected-error 1+{{definition of 'B' must be imported from module 'M:B' before it is required}}
- // expected-note@* 1+{{definition here is not reachable}}
+void g1() { f(B()); } // expected-error 1+{{definition of 'B' is private to module 'M:B'}}
+ // expected-note@* 1+{{export the definition to make it available}}
// expected-n...@m.cppm:5 {{passing argument to parameter 'b' here}}
Index: clang/test/CXX/module/module.interface/p7.cpp
===================================================================
--- clang/test/CXX/module/module.interface/p7.cpp
+++ clang/test/CXX/module/module.interface/p7.cpp
@@ -57,12 +57,12 @@
void test2() {
auto a = E1::e1; // OK, namespace-scope name E1 is visible and e1 is reachable
auto b = e1; // OK, namespace-scope name e1 is visible
- auto c = E2::e2; // expected-error {{declaration of 'E2' must be imported from module}}
- // expected-note@* {{declaration here is not visible}}
+ auto c = E2::e2; // expected-error {{declaration of 'E2' is private to module 'p7'}}
+ // expected-note@* {{export the declaration to make it available}}
auto d = e2; // should be error, namespace-scope name e2 is not visible
auto e = E2U::e2; // OK, namespace-scope name E2U is visible and E2::e2 is reachable
- auto f = E3::e3; // expected-error {{declaration of 'E3' must be imported from module 'p7' before it is required}}
- // expected-note@* {{declaration here is not visible}}
+ auto f = E3::e3; // expected-error {{declaration of 'E3' is private to module 'p7'}}
+ // expected-note@* {{export the declaration to make it available}}
auto g = e3; // should be error, namespace-scope name e3 is not visible
auto h = decltype(func())::e3; // OK, namespace-scope name f is visible and E3::e3 is reachable
}
Index: clang/test/CXX/module/module.interface/p2.cpp
===================================================================
--- clang/test/CXX/module/module.interface/p2.cpp
+++ clang/test/CXX/module/module.interface/p2.cpp
@@ -69,29 +69,29 @@
void use() {
// namespace A is implicitly exported by the export of A::g.
- A::f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
- // expected-note@* {{declaration here is not visible}}
+ A::f(); // expected-error {{declaration of 'f' is private to module 'p2'}}
+ // expected-note@* {{export the declaration to make it available}}
A::g();
- A::h(); // expected-error {{declaration of 'h' must be imported from module 'p2' before it is required}}
- // expected-note@* {{declaration here is not visible}}
+ A::h(); // expected-error {{declaration of 'h' is private to module 'p2'}}
+ // expected-note@* {{export the declaration to make it available}}
using namespace A::inner; // expected-error {{declaration of 'inner' must be imported from module 'p2' before it is required}}
// expected-note@* {{declaration here is not visible}}
// namespace B and B::inner are explicitly exported
using namespace B;
using namespace B::inner;
- B::f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
- // expected-note@* {{declaration here is not visible}}
- f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
- // expected-note@* {{declaration here is not visible}}
+ B::f(); // expected-error {{declaration of 'f' is private to module 'p2'}}
+ // expected-note@* {{export the declaration to make it available}}
+ f(); // expected-error {{declaration of 'f' is private to module 'p2'}}
+ // expected-note@* {{export the declaration to make it available}}
// namespace C is not exported
using namespace C; // expected-error {{declaration of 'C' must be imported from module 'p2' before it is required}}
// expected-note@* {{declaration here is not visible}}
// namespace D is exported, but D::f is not
- D::f(); // expected-error {{declaration of 'f' must be imported from module 'p2' before it is required}}
- // expected-note@* {{declaration here is not visible}}
+ D::f(); // expected-error {{declaration of 'f' is private to module 'p2'}}
+ // expected-note@* {{export the declaration to make it available}}
}
int use_header() { return foo + bar::baz(); }
Index: clang/test/CXX/module/module.import/p2.cpp
===================================================================
--- clang/test/CXX/module/module.import/p2.cpp
+++ clang/test/CXX/module/module.import/p2.cpp
@@ -23,10 +23,10 @@
//--- 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 {{}}
- // 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 {{}}
+ A a; // expected-error {{declaration of 'A' is private to module 'M:impl'}}
+ // expected-n...@impl.cppm:2 {{export the declaration to make it available}}
+ // expected-error@-2 1+{{definition of 'A' is private to module 'M:impl'}}
+ // expected-n...@impl.cppm:2 1+{{export the definition to make it available}}
}
//--- UseInPartA.cppm
@@ -41,10 +41,10 @@
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 {{}}
- // 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 {{}}
+ A a; // expected-error {{declaration of 'A' is private to module 'M:impl'}}
+ // expected-n...@impl.cppm:2 {{export the declaration to make it available}}
+ // expected-error@-2 1+{{definition of 'A' is private to module 'M:impl'}}
+ // expected-n...@impl.cppm:2 1+{{export the definition to make it available}}
}
//--- Private.cppm
Index: clang/test/CXX/module/basic/basic.link/p2.cppm
===================================================================
--- clang/test/CXX/module/basic/basic.link/p2.cppm
+++ clang/test/CXX/module/basic/basic.link/p2.cppm
@@ -39,20 +39,19 @@
}
//--- M.cpp
-// expected-no-diagnostics
module M;
-// FIXME: Use of internal linkage entities should be rejected.
void use_from_module_impl() {
external_linkage_fn();
module_linkage_fn();
- internal_linkage_fn();
+ internal_linkage_fn(); // expected-error {{use of undeclared identifier 'internal_linkage_fn'; did you mean 'external_linkage_fn'}}
+ // expected-n...@m.cppm:8 {{'external_linkage_fn' declared here}}
(void)external_linkage_class{};
(void)module_linkage_class{};
- (void)internal_linkage_class{};
+ (void)internal_linkage_class; // expected-error {{use of undeclared identifier 'internal_linkage_class'}}
(void)external_linkage_var;
(void)module_linkage_var;
- (void)internal_linkage_var;
+ (void)internal_linkage_var; // expected-error {{use of undeclared identifier 'internal_linkage_var'}}
}
//--- user.cpp
@@ -60,13 +59,13 @@
void use_from_module_impl() {
external_linkage_fn();
- module_linkage_fn(); // expected-error {{declaration of 'module_linkage_fn' must be imported}}
- internal_linkage_fn(); // expected-error {{declaration of 'internal_linkage_fn' must be imported}}
+ module_linkage_fn(); // expected-error {{declaration of 'module_linkage_fn' is private to module 'M'}}
+ // expected-n...@m.cppm:9 {{export the declaration to make it available}}
+ internal_linkage_fn(); // expected-error {{use of undeclared identifier 'internal_linkage_fn'; did you mean 'external_linkage_fn'}}
(void)external_linkage_class{};
(void)module_linkage_class{}; // expected-error {{undeclared identifier}} expected-error 0+{{}}
(void)internal_linkage_class{}; // expected-error {{undeclared identifier}} expected-error 0+{{}}
- // expected-n...@m.cppm:9 {{declaration here is not visible}}
- // expected-n...@m.cppm:10 {{declaration here is not visible}}
+ // expected-n...@m.cppm:8 {{'external_linkage_fn' declared here}}
(void)external_linkage_var;
(void)module_linkage_var; // expected-error {{undeclared identifier}}
(void)internal_linkage_var; // expected-error {{undeclared identifier}}
Index: clang/test/CXX/module/basic/basic.def.odr/p4.cppm
===================================================================
--- clang/test/CXX/module/basic/basic.def.odr/p4.cppm
+++ clang/test/CXX/module/basic/basic.def.odr/p4.cppm
@@ -5,9 +5,9 @@
// RUN: %clang_cc1 -std=c++20 %t/Module.cppm -triple %itanium_abi_triple -emit-llvm -o - | FileCheck %t/Module.cppm --implicit-check-not unused
//
// RUN: %clang_cc1 -std=c++20 %t/Module.cppm -triple %itanium_abi_triple -emit-module-interface -o %t/Module.pcm
-// RUN: %clang_cc1 -std=c++20 %t/module.cpp -triple %itanium_abi_triple -fmodule-file=%t/Module.pcm -emit-llvm -o - | FileCheck %t/module.cpp --implicit-check-not=unused --implicit-check-not=global_module
+// RUN: %clang_cc1 -std=c++20 %t/module.cpp -triple %itanium_abi_triple -fmodule-file=Module=%t/Module.pcm -emit-llvm -o - | FileCheck %t/module.cpp --implicit-check-not=unused --implicit-check-not=global_module
//
-// RUN: %clang_cc1 -std=c++20 %t/user.cpp -triple %itanium_abi_triple -fmodule-file=%t/Module.pcm -emit-llvm -o - | FileCheck %t/user.cpp --implicit-check-not=unused --implicit-check-not=global_module
+// RUN: %clang_cc1 -std=c++20 %t/user.cpp -triple %itanium_abi_triple -fmodule-file=Module=%t/Module.pcm -emit-llvm -o - | FileCheck %t/user.cpp --implicit-check-not=unused --implicit-check-not=global_module
//--- Module.cppm
// CHECK-DAG: @extern_var_global_module = external {{(dso_local )?}}global
@@ -128,8 +128,6 @@
//
// CHECK-DAG: @_ZW6Module25extern_var_module_linkage = external {{(dso_local )?}}global
// CHECK-DAG: @_ZW6Module25inline_var_module_linkage = linkonce_odr {{(dso_local )?}}global
-// CHECK-DAG: @_ZL25static_var_module_linkage = internal {{(dso_local )?}}global i32 0,
-// CHECK-DAG: @_ZL24const_var_module_linkage = internal {{(dso_local )?}}constant i32 3,
module Module;
@@ -143,9 +141,6 @@
(void)&inline_var_exported;
(void)&const_var_exported;
- // CHECK: define {{.*}}@_ZL26used_static_module_linkagev
- used_static_module_linkage();
-
// CHECK: define linkonce_odr {{.*}}@_ZW6Module26used_inline_module_linkagev
used_inline_module_linkage();
@@ -154,8 +149,7 @@
(void)&extern_var_module_linkage;
(void)&inline_var_module_linkage;
- (void)&static_var_module_linkage; // FIXME: Should not be visible here.
- (void)&const_var_module_linkage;
+ // Internal linkage declarations are not visible here.
}
//--- user.cpp
@@ -176,5 +170,6 @@
(void)&inline_var_exported;
(void)&const_var_exported;
+ // Internal linkage declarations are not visible here.
// Module-linkage declarations are not visible here.
}
Index: clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
===================================================================
--- clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
+++ clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp
@@ -62,27 +62,15 @@
not_exported = 1;
#ifndef IMPLEMENTATION
- // expected-error@-2 {{declaration of 'not_exported' must be imported from module 'A' before it is required}}
- // expected-n...@p2.cpp:19 {{declaration here is not visible}}
+ // expected-error@-2 {{declaration of 'not_exported' is private to module 'A'}}
+ // expected-n...@p2.cpp:19 {{export the declaration to make it available}}
#endif
- internal = 1;
-#ifndef IMPLEMENTATION
- // expected-error@-2 {{declaration of 'internal' must be imported from module 'A' before it is required}}
- // expected-n...@p2.cpp:20 {{declaration here is not visible}}
-#endif
+ internal = 1; // expected-error {{use of undeclared identifier 'internal'}}
- not_exported_private = 1;
-#ifndef IMPLEMENTATION
- // FIXME: should not be visible here
- // expected-error@-3 {{undeclared identifier}}
-#endif
+ not_exported_private = 1; // expected-error {{use of undeclared identifier 'not_exported_private'}}
- internal_private = 1;
-#ifndef IMPLEMENTATION
- // FIXME: should not be visible here
- // expected-error@-3 {{undeclared identifier}}
-#endif
+ internal_private = 1; // expected-error {{undeclared identifier 'internal_private'}}
}
#endif
Index: clang/lib/Sema/SemaModule.cpp
===================================================================
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -301,6 +301,8 @@
Module *Mod;
switch (MDK) {
+ case ModuleDeclKind::NoModule:
+ llvm_unreachable("we should be building some kind of module");
case ModuleDeclKind::Interface:
case ModuleDeclKind::PartitionInterface: {
// We can't have parsed or imported a definition of this module or parsed a
@@ -385,6 +387,7 @@
// We are in the module purview, but before any other (non import)
// statements, so imports are allowed.
ImportState = ModuleImportState::ImportAllowed;
+ TheModuleKind = MDK; // Save this information to help decl TU queries.
// For an implementation, We already made an implicit import (its interface).
// Make and return the import decl to be added to the current TU.
@@ -1014,10 +1017,42 @@
Module *CurrentModuleUnit = getCurrentModule();
- // If we are not in a module currently, M must not be the module unit of
+ // If we are not in a module currently, M cannot be a module unit of the
// current TU.
if (!CurrentModuleUnit)
return false;
+ // We only create the special GlobalModuleFragments for the current TU; GMF
+ // fragments of imported modules are part of those imports. These are the
+ // only sub-modules (potentially) present for a non-partition module
+ // implementation TU.
+ if (M == TheGlobalModuleFragment || M == TheImplicitGlobalModuleFragment ||
+ M == TheExportedImplicitGlobalModuleFragment)
+ return true;
+
+ // If we are in the GMF and the match above failed, the query is for some
+ // other TU.
+ if (CurrentModuleUnit == TheGlobalModuleFragment)
+ return false;
+
+ // If we are in a non-partition module implementation TU, then we are done;
+ // the only possible cases are considered above.
+ if (TheModuleKind == ModuleDeclKind::Implementation)
+ return false;
+
+ // Now we've checked for the implementation case, we could have a query for
+ // a decl earlier in the current module purview.
+ if (M == CurrentModuleUnit)
+ return true;
+
+ // If we are (temporarily) in one of the special Global Module fragments that
+ // handle language linkage cases, then the only relevant case is that the
+ // parent equals the queried module.
+ if (CurrentModuleUnit == TheImplicitGlobalModuleFragment ||
+ CurrentModuleUnit == TheExportedImplicitGlobalModuleFragment) {
+ return CurrentModuleUnit->Parent == M;
+ }
+
+ // FIXME: probably, it is sufficient to check for the PMF here.
return M->isSubModuleOf(CurrentModuleUnit->getTopLevelModule());
}
Index: clang/lib/Sema/SemaLookup.cpp
===================================================================
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1780,7 +1780,7 @@
Module *DeclModule = SemaRef.getOwningModule(D);
assert(DeclModule && "hidden decl has no owning module");
- // If the owning module is visible, the decl is acceptable.
+ // If the owning module is visible, the decl is potentially acceptable.
if (SemaRef.isModuleVisible(DeclModule,
D->isInvisibleOutsideTheOwningModule()))
return true;
@@ -2065,14 +2065,27 @@
return findAcceptableDecl(getSema(), D, IDNS);
}
-bool LookupResult::isVisible(Sema &SemaRef, NamedDecl *D) {
+bool LookupResult::isVisible(Sema &SemaRef, NamedDecl *D, bool ForTypo) {
// If this declaration is already visible, return it directly.
if (D->isUnconditionallyVisible())
return true;
// During template instantiation, we can refer to hidden declarations, if
// they were visible in any module along the path of instantiation.
- return isAcceptableSlow(SemaRef, D, Sema::AcceptableKind::Visible);
+ if (!isAcceptableSlow(SemaRef, D, Sema::AcceptableKind::Visible))
+ return false;
+
+ if (ForTypo && SemaRef.getLangOpts().CPlusPlusModules && D->isFromASTFile()) {
+ // There is no point in offering a typo correction to an item that we will
+ // reject because it has internal linkage and is in a different module TU;
+ Module *DeclModule = SemaRef.getOwningModule(D);
+ if (DeclModule && !DeclModule->isModuleMapModule() &&
+ !SemaRef.isModuleUnitOfCurrentTU(DeclModule) &&
+ (D->getFormalLinkage() == Linkage::InternalLinkage ||
+ DeclModule->isPrivateModule()))
+ return false; // This is not usefully visible.
+ }
+ return true;
}
bool LookupResult::isReachable(Sema &SemaRef, NamedDecl *D) {
@@ -2084,8 +2097,22 @@
bool LookupResult::isAvailableForLookup(Sema &SemaRef, NamedDecl *ND) {
// We should check the visibility at the callsite already.
- if (isVisible(SemaRef, ND))
+ if (isVisible(SemaRef, ND)) {
+ if (SemaRef.getLangOpts().CPlusPlusModules && ND->isFromASTFile()) {
+ // The module that owns the decl is visible; However
+ // [basic.lookup.general]/2, specifically 2.3
+ // If the owning module is part of a different TU than the current one,
+ // and declaration is internal (or from the PMF), then we should not
+ // include it.
+ Module *DeclModule = SemaRef.getOwningModule(ND);
+ if (DeclModule && !DeclModule->isModuleMapModule() &&
+ !SemaRef.isModuleUnitOfCurrentTU(DeclModule) &&
+ (ND->getFormalLinkage() == Linkage::InternalLinkage ||
+ DeclModule->isPrivateModule()))
+ return false;
+ }
return true;
+ }
// Deduction guide lives in namespace scope generally, but it is just a
// hint to the compilers. What we actually lookup for is the generated member
@@ -4463,9 +4490,13 @@
static void checkCorrectionVisibility(Sema &SemaRef, TypoCorrection &TC) {
TypoCorrection::decl_iterator DI = TC.begin(), DE = TC.end();
- for (/**/; DI != DE; ++DI)
- if (!LookupResult::isVisible(SemaRef, *DI))
+ // During the lookups for Typo corrections, we allow 'hidden' decls to be
+ // found, this can also include (potential typo correction) decls that are
+ // not in their own right viable. Filter these from the visible set.
+ for (/**/; DI != DE; ++DI) {
+ if (!LookupResult::isVisible(SemaRef, *DI, /*ForTypo*/ true))
break;
+ }
// No filtering needed if all decls are visible.
if (DI == DE) {
TC.setRequiresImport(false);
@@ -4476,15 +4507,25 @@
bool AnyVisibleDecls = !NewDecls.empty();
for (/**/; DI != DE; ++DI) {
- if (LookupResult::isVisible(SemaRef, *DI)) {
+ if (LookupResult::isVisible(SemaRef, *DI, /*ForTypo*/ true)) {
if (!AnyVisibleDecls) {
// Found a visible decl, discard all hidden ones.
AnyVisibleDecls = true;
NewDecls.clear();
}
NewDecls.push_back(*DI);
- } else if (!AnyVisibleDecls && !(*DI)->isModulePrivate())
- NewDecls.push_back(*DI);
+ } else if (!AnyVisibleDecls) {
+ // We do not want to push a hidden decl if it is not usefully reported
+ // as a typo (which means the proposed replacement has to be viable).
+ NamedDecl *ND = *DI;
+ if (SemaRef.getLangOpts().CPlusPlusModules && ND->isFromASTFile() &&
+ ND->getOwningModule() &&
+ !SemaRef.isModuleUnitOfCurrentTU(ND->getOwningModule()) &&
+ ND->getFormalLinkage() == Linkage::InternalLinkage)
+ continue; // Internal symbol from a different TU, not visible.
+ if (!ND->isModulePrivate())
+ NewDecls.push_back(ND);
+ }
}
if (NewDecls.empty())
@@ -4552,7 +4593,7 @@
// Only consider visible declarations and declarations from modules with
// names that exactly match.
- if (!LookupResult::isVisible(SemaRef, ND) && Name != Typo)
+ if (!LookupResult::isVisible(SemaRef, ND, /*ForTypo*/ true) && Name != Typo)
return;
FoundName(Name->getName());
@@ -5735,6 +5776,7 @@
Modules = UniqueModules;
+ bool DidFixit = false;
if (Modules.size() > 1) {
std::string ModuleList;
unsigned N = 0;
@@ -5749,13 +5791,26 @@
Diag(UseLoc, diag::err_module_unimported_use_multiple)
<< (int)MIK << Decl << ModuleList;
+ } else if (Decl->hasLinkage() &&
+ Decl->getFormalLinkage() == Linkage::ModuleLinkage) {
+ Diag(UseLoc, diag::err_module_private_use)
+ << (int)MIK << Decl << Modules[0]->getFullModuleName();
+ Diag(Decl->getBeginLoc(), diag::note_suggest_export)
+ << (int)MIK
+ << FixItHint::CreateInsertion(Decl->getBeginLoc(), "export");
+ DidFixit = true;
+ } else if (Decl->hasLinkage() &&
+ Decl->getFormalLinkage() == Linkage::InternalLinkage) {
+ Diag(UseLoc, diag::err_module_internal_use)
+ << (int)MIK << Decl << Modules[0]->getFullModuleName();
} else {
// FIXME: Add a FixItHint that imports the corresponding module.
Diag(UseLoc, diag::err_module_unimported_use)
<< (int)MIK << Decl << Modules[0]->getFullModuleName();
}
- NotePrevious();
+ if (!DidFixit)
+ NotePrevious();
// Try to recover by implicitly importing this module.
if (Recover)
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -3141,12 +3141,16 @@
SourceLocation SemiLoc);
enum class ModuleDeclKind {
+ NoModule, ///< No named module line seen so far.
Interface, ///< 'export module X;'
Implementation, ///< 'module X;'
PartitionInterface, ///< 'export module X:Y;'
PartitionImplementation, ///< 'module X:Y;'
};
+ /// Identify the module kind to disambiguate implementation units.
+ ModuleDeclKind TheModuleKind = ModuleDeclKind::NoModule;
+
/// An enumeration to represent the transition of states in parsing module
/// fragments and imports. If we are not parsing a C++20 TU, or we find
/// an error in state transition, the state is set to NotACXX20Module.
Index: clang/include/clang/Sema/Lookup.h
===================================================================
--- clang/include/clang/Sema/Lookup.h
+++ clang/include/clang/Sema/Lookup.h
@@ -346,7 +346,7 @@
/// Determine whether the given declaration is visible to the
/// program.
- static bool isVisible(Sema &SemaRef, NamedDecl *D);
+ static bool isVisible(Sema &SemaRef, NamedDecl *D, bool ForTypo = false);
static bool isReachable(Sema &SemaRef, NamedDecl *D);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11167,6 +11167,17 @@
"%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_private_use : Error<
+ "%select{declaration|definition|default argument|"
+ "explicit specialization|partial specialization}0 of %1 is private to module "
+ "'%2'">;
+def err_module_internal_use : Error<
+ "%select{declaration|definition|default argument|"
+ "explicit specialization|partial specialization}0 of %1 is internal to "
+ "'%2'">;
+def note_suggest_export : Note<
+ "export the %select{declaration|definition|default argument|"
+ "explicit specialization|partial specialization}0 to make it available">;
def err_module_unimported_use_header : Error<
"%select{missing '#include'|missing '#include %3'}2; "
"%select{||default argument of |explicit specialization of |"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
