Skip to content

Commit

Permalink
[C++20] [Modules] Don't generate unused variables in other module units
Browse files Browse the repository at this point in the history
even if its initializer has side effects

Close #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.
  • Loading branch information
ChuanqiXu9 committed May 10, 2023
1 parent 7591a7b commit b6c7177
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 12 deletions.
2 changes: 1 addition & 1 deletion clang/include/clang/AST/DeclBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
12 changes: 7 additions & 5 deletions clang/lib/AST/DeclBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Sema/SemaLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
/// different from the semantics in C++ spec. In C++ spec, only names
/// have linkage. So that all entities of the same should share one
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/Sema/SemaTemplate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)) {
Expand Down
65 changes: 65 additions & 0 deletions clang/test/Modules/pr61892.cppm
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit b6c7177

Please sign in to comment.