[clang] b6c7177 - [C++20] [Modules] Don't generate unused variables in other module units
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Wed May 10 02:42:15 PDT 2023
Author: Chuanqi Xu
Date: 2023-05-10T17:41:58+08:00
New Revision: b6c7177145bc439c712208bfbe041db212c5262d
URL: https://github.com/llvm/llvm-project/commit/b6c7177145bc439c712208bfbe041db212c5262d
DIFF: https://github.com/llvm/llvm-project/commit/b6c7177145bc439c712208bfbe041db212c5262d.diff
LOG: [C++20] [Modules] Don't generate unused variables in other module units
even if its initializer has side effects
Close https://github.com/llvm/llvm-project/issues/61892
The variables whose initializer has side effects will be emitted even if
it is not used. But it shouldn't be true after we introduced modules.
The variables in other modules shouldn't be emitted if it is not used
even if its initializer has size effects.
Also this patch rename `Decl::isInCurrentModuleUnit` to
`Decl::isInAnotherModuleUnit` to make it closer to the semantics.
Added:
clang/test/Modules/pr61892.cppm
Modified:
clang/include/clang/AST/DeclBase.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 571f1f10387bf..65215dbf70551 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -645,7 +645,7 @@ class alignas(8) Decl {
}
/// Whether this declaration comes from another module unit.
- bool isInCurrentModuleUnit() const;
+ bool isInAnotherModuleUnit() const;
/// FIXME: Implement discarding declarations actually in global module
/// fragment. See [module.global.frag]p3,4 for details.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 3441c069b4b83..b9919073e8267 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -11926,6 +11926,10 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
!isMSStaticDataMemberInlineDefinition(VD))
return false;
+ // Variables in other module units shouldn't be forced to be emitted.
+ if (VD->isInAnotherModuleUnit())
+ return false;
+
// Variables that can be needed in other TUs are required.
auto Linkage = GetGVALinkageForVariable(VD);
if (!isDiscardableGVALinkage(Linkage))
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index fe458b67dcab0..834beef49a444 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1023,24 +1023,26 @@ bool Decl::isInExportDeclContext() const {
return DC && isa<ExportDecl>(DC);
}
-bool Decl::isInCurrentModuleUnit() const {
+bool Decl::isInAnotherModuleUnit() const {
auto *M = getOwningModule();
if (!M)
- return true;
+ return false;
M = M->getTopLevelModule();
// FIXME: It is problematic if the header module lives in another module
// unit. Consider to fix this by techniques like
// ExternalASTSource::hasExternalDefinitions.
if (M->isHeaderLikeModule())
- return true;
+ return false;
+ // A global module without parent implies that we're parsing the global
+ // module. So it can't be in another module unit.
if (M->isGlobalModule())
- return true;
+ return false;
assert(M->isModulePurview() && "New module kind?");
- return M == getASTContext().getCurrentNamedModule();
+ return M != getASTContext().getCurrentNamedModule();
}
static Decl::Kind getKind(const Decl *D) { return D->getKind(); }
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index b59ee727d267b..6621ebddf58d0 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -1909,7 +1909,7 @@ bool LookupResult::isReachableSlow(Sema &SemaRef, NamedDecl *D) {
if (DeclModule->isHeaderLikeModule())
return false;
- if (D->isInCurrentModuleUnit())
+ if (!D->isInAnotherModuleUnit())
return true;
// [module.reach]/p3:
@@ -3889,7 +3889,7 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, SourceLocation Loc,
"bad export context");
// .. are attached to a named module M, do not appear in the
// translation unit containing the point of the lookup..
- if (!D->isInCurrentModuleUnit() &&
+ if (D->isInAnotherModuleUnit() &&
llvm::any_of(AssociatedClasses, [&](auto *E) {
// ... and have the same innermost enclosing non-inline
// namespace scope as a declaration of an associated entity
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 419adff028c28..7f3e78c89f57a 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -6543,7 +6543,7 @@ void Sema::AddOverloadCandidate(
}
// Functions with internal linkage are only viable in the same module unit.
- if (getLangOpts().CPlusPlusModules && !Function->isInCurrentModuleUnit()) {
+ if (getLangOpts().CPlusPlusModules && Function->isInAnotherModuleUnit()) {
/// FIXME: Currently, the semantics of linkage in clang is slightly
///
diff erent from the semantics in C++ spec. In C++ spec, only names
/// have linkage. So that all entities of the same should share one
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index bd0aa8edb11fb..b3a180e909b3f 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2851,7 +2851,7 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
NewDefaultLoc = NewTypeParm->getDefaultArgumentLoc();
SawDefaultArgument = true;
- if (OldTypeParm->isInCurrentModuleUnit())
+ if (!OldTypeParm->isInAnotherModuleUnit())
RedundantDefaultArg = true;
else if (!getASTContext().isSameDefaultTemplateArgument(OldTypeParm,
NewTypeParm)) {
@@ -2903,7 +2903,7 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc();
NewDefaultLoc = NewNonTypeParm->getDefaultArgumentLoc();
SawDefaultArgument = true;
- if (OldNonTypeParm->isInCurrentModuleUnit())
+ if (!OldNonTypeParm->isInAnotherModuleUnit())
RedundantDefaultArg = true;
else if (!getASTContext().isSameDefaultTemplateArgument(
OldNonTypeParm, NewNonTypeParm)) {
@@ -2954,7 +2954,7 @@ bool Sema::CheckTemplateParameterList(TemplateParameterList *NewParams,
OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation();
NewDefaultLoc = NewTemplateParm->getDefaultArgument().getLocation();
SawDefaultArgument = true;
- if (OldTemplateParm->isInCurrentModuleUnit())
+ if (!OldTemplateParm->isInAnotherModuleUnit())
RedundantDefaultArg = true;
else if (!getASTContext().isSameDefaultTemplateArgument(
OldTemplateParm, NewTemplateParm)) {
diff --git a/clang/test/Modules/pr61892.cppm b/clang/test/Modules/pr61892.cppm
new file mode 100644
index 0000000000000..99d02f36b2b54
--- /dev/null
+++ b/clang/test/Modules/pr61892.cppm
@@ -0,0 +1,65 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple \
+// RUN: -emit-module-interface %t/a.cppm -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple \
+// RUN: %t/b.cpp -fmodule-file=a=%t/a.pcm -disable-llvm-passes \
+// RUN: -emit-llvm -o - | FileCheck %t/b.cpp
+// RUN: %clang_cc1 -std=c++20 -triple %itanium_abi_triple \
+// RUN: %t/c.cpp -fmodule-file=a=%t/a.pcm -disable-llvm-passes \
+// RUN: -emit-llvm -o - | FileCheck %t/c.cpp
+
+//--- a.cppm
+export module a;
+
+struct integer {
+ explicit operator int() const {
+ return 0;
+ }
+};
+
+export template<typename>
+int a = static_cast<int>(integer());
+
+struct s {
+ ~s();
+ operator int() const;
+};
+
+export template<typename>
+auto d = s();
+
+int aa() {
+ return a<void> + d<void>;
+}
+
+int dynamic_func();
+export inline int dynamic_var = dynamic_func();
+
+//--- b.cpp
+import a;
+
+void b() {}
+
+// CHECK-NOT: @_ZW1a1dIvE =
+// CHECK-NOT: @_ZGVW1a1dIvE =
+// CHECK-NOT: @_ZW1a11dynamic_var =
+// CHECK-NOT: @_ZGVW1a11dynamic_var =
+// CHECK-NOT: @_ZW1a1aIvE =
+// CHECK-NOT: @_ZGVW1a1aIvE =
+
+//--- c.cpp
+import a;
+int c() {
+ return a<void> + d<void> + dynamic_var;
+}
+
+// The used variables are generated normally
+// CHECK-DAG: @_ZW1a1aIvE =
+// CHECK-DAG: @_ZW1a1dIvE =
+// CHECK-DAG: @_ZW1a11dynamic_var = linkonce_odr
+// CHECK-DAG: @_ZGVW1a1aIvE =
+// CHECk-DAG: @_ZGVW1a1dIvE =
+// CHECK-DAG: @_ZGVW1a11dynamic_var = linkonce_odr
More information about the cfe-commits
mailing list