Skip to content

[Modules] fix assert in hasInitWithSideEffects #146468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions clang/include/clang/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -888,13 +888,17 @@ struct EvaluatedStmt {
bool HasICEInit : 1;
bool CheckedForICEInit : 1;

bool HasSideEffects : 1;
bool CheckedForSideEffects : 1;

LazyDeclStmtPtr Value;
APValue Evaluated;

EvaluatedStmt()
: WasEvaluated(false), IsEvaluating(false),
HasConstantInitialization(false), HasConstantDestruction(false),
HasICEInit(false), CheckedForICEInit(false) {}
HasICEInit(false), CheckedForICEInit(false), HasSideEffects(false),
CheckedForSideEffects(false) {}
};

/// Represents a variable declaration or definition.
Expand Down Expand Up @@ -1353,9 +1357,11 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
return const_cast<VarDecl *>(this)->getInitializingDeclaration();
}

/// Checks whether this declaration has an initializer with side effects,
/// without triggering deserialization if the initializer is not yet
/// deserialized.
/// Checks whether this declaration has an initializer with side effects.
/// The result is cached. If the result hasn't been computed this can trigger
/// deserialization and constant evaluation. By running this during
/// serialization and serializing the result all clients can safely call this
/// without triggering further deserialization.
bool hasInitWithSideEffects() const;

/// Determine whether this variable's value might be usable in a
Expand Down
15 changes: 0 additions & 15 deletions clang/include/clang/AST/ExternalASTSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,6 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
/// module.
virtual bool wasThisDeclarationADefinition(const FunctionDecl *FD);

virtual bool hasInitializerWithSideEffects(const VarDecl *VD) const {
return false;
}

/// Finds all declarations lexically contained within the given
/// DeclContext, after applying an optional filter predicate.
///
Expand Down Expand Up @@ -434,17 +430,6 @@ struct LazyOffsetPtr {
return GetPtr();
}

/// Retrieve the pointer to the AST node that this lazy pointer points to,
/// if it can be done without triggering deserialization.
///
/// \returns a pointer to the AST node, or null if not yet deserialized.
T *getWithoutDeserializing() const {
if (isOffset()) {
return nullptr;
}
return GetPtr();
}

/// Retrieve the address of the AST node pointer. Deserializes the pointee if
/// necessary.
T **getAddressOfPointer(ExternalASTSource *Source) const {
Expand Down
2 changes: 0 additions & 2 deletions clang/include/clang/Sema/MultiplexExternalSemaSource.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,6 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {

bool wasThisDeclarationADefinition(const FunctionDecl *FD) override;

bool hasInitializerWithSideEffects(const VarDecl *VD) const override;

/// Find all declarations with the given name in the
/// given context.
bool FindExternalVisibleDeclsByName(const DeclContext *DC,
Expand Down
8 changes: 0 additions & 8 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -1453,12 +1453,6 @@ class ASTReader
const StringRef &operator*() && = delete;
};

/// VarDecls with initializers containing side effects must be emitted,
/// but DeclMustBeEmitted is not allowed to deserialize the intializer.
/// FIXME: Lower memory usage by removing VarDecls once the initializer
/// is deserialized.
llvm::SmallPtrSet<Decl *, 16> InitSideEffectVars;

public:
/// Get the buffer for resolving paths.
SmallString<0> &getPathBuf() { return PathBuf; }
Expand Down Expand Up @@ -2410,8 +2404,6 @@ class ASTReader

bool wasThisDeclarationADefinition(const FunctionDecl *FD) override;

bool hasInitializerWithSideEffects(const VarDecl *VD) const override;

/// Retrieve a selector from the given module with its local ID
/// number.
Selector getLocalSelector(ModuleFile &M, unsigned LocalID);
Expand Down
28 changes: 10 additions & 18 deletions clang/lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2444,24 +2444,16 @@ bool VarDecl::hasInitWithSideEffects() const {
if (!hasInit())
return false;

// Check if we can get the initializer without deserializing
const Expr *E = nullptr;
if (auto *S = dyn_cast<Stmt *>(Init)) {
E = cast<Expr>(S);
} else {
E = cast_or_null<Expr>(getEvaluatedStmt()->Value.getWithoutDeserializing());
}

if (E)
return E->HasSideEffects(getASTContext()) &&
// We can get a value-dependent initializer during error recovery.
(E->isValueDependent() || !evaluateValue());

assert(getEvaluatedStmt()->Value.isOffset());
// ASTReader tracks this without having to deserialize the initializer
if (auto Source = getASTContext().getExternalSource())
return Source->hasInitializerWithSideEffects(this);
return false;
EvaluatedStmt *ES = ensureEvaluatedStmt();
if (!ES->CheckedForSideEffects) {
const Expr *E = getInit();
ES->HasSideEffects =
E->HasSideEffects(getASTContext()) &&
// We can get a value-dependent initializer during error recovery.
(E->isValueDependent() || !evaluateValue());
ES->CheckedForSideEffects = true;
}
return ES->HasSideEffects;
}

bool VarDecl::isOutOfLine() const {
Expand Down
8 changes: 0 additions & 8 deletions clang/lib/Sema/MultiplexExternalSemaSource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,6 @@ bool MultiplexExternalSemaSource::wasThisDeclarationADefinition(
return false;
}

bool MultiplexExternalSemaSource::hasInitializerWithSideEffects(
const VarDecl *VD) const {
for (const auto &S : Sources)
if (S->hasInitializerWithSideEffects(VD))
return true;
return false;
}

bool MultiplexExternalSemaSource::FindExternalVisibleDeclsByName(
const DeclContext *DC, DeclarationName Name,
const DeclContext *OriginalDC) {
Expand Down
4 changes: 0 additions & 4 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9725,10 +9725,6 @@ bool ASTReader::wasThisDeclarationADefinition(const FunctionDecl *FD) {
return ThisDeclarationWasADefinitionSet.contains(FD);
}

bool ASTReader::hasInitializerWithSideEffects(const VarDecl *VD) const {
return InitSideEffectVars.count(VD);
}

Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
return DecodeSelector(getGlobalSelectorID(M, LocalID));
}
Expand Down
5 changes: 2 additions & 3 deletions clang/lib/Serialization/ASTReaderDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1628,9 +1628,6 @@ RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
VD->NonParmVarDeclBits.PreviousDeclInSameBlockScope =
VarDeclBits.getNextBit();

if (VarDeclBits.getNextBit())
Reader.InitSideEffectVars.insert(VD);

VD->NonParmVarDeclBits.EscapingByref = VarDeclBits.getNextBit();
HasDeducedType = VarDeclBits.getNextBit();
VD->NonParmVarDeclBits.ImplicitParamKind =
Expand Down Expand Up @@ -1701,6 +1698,8 @@ void ASTDeclReader::ReadVarDeclInit(VarDecl *VD) {
Eval->HasConstantInitialization = (Val & 2) != 0;
Eval->HasConstantDestruction = (Val & 4) != 0;
Eval->WasEvaluated = (Val & 8) != 0;
Eval->HasSideEffects = (Val & 16) != 0;
Eval->CheckedForSideEffects = true;
if (Eval->WasEvaluated) {
Eval->Evaluated = Record.readAPValue();
if (Eval->Evaluated.needsCleanup())
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7320,6 +7320,10 @@ void ASTRecordWriter::AddVarDeclInit(const VarDecl *VD) {

uint64_t Val = 1;
if (EvaluatedStmt *ES = VD->getEvaluatedStmt()) {
// This may trigger evaluation, so run it first
if (VD->hasInitWithSideEffects())
Val |= 16;
assert(ES->CheckedForSideEffects);
Val |= (ES->HasConstantInitialization ? 2 : 0);
Val |= (ES->HasConstantDestruction ? 4 : 0);
APValue *Evaluated = VD->getEvaluatedValue();
Expand Down
1 change: 0 additions & 1 deletion clang/lib/Serialization/ASTWriterDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1305,7 +1305,6 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
VarDeclBits.addBit(D->isConstexpr());
VarDeclBits.addBit(D->isInitCapture());
VarDeclBits.addBit(D->isPreviousDeclInSameBlockScope());
VarDeclBits.addBit(D->hasInitWithSideEffects());

VarDeclBits.addBit(D->isEscapingByref());
HasDeducedType = D->getType()->getContainedDeducedType();
Expand Down
51 changes: 51 additions & 0 deletions clang/test/Modules/var-init-side-effects-modulemap.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// RUN: rm -rf %t
// RUN: mkdir -p %t
// RUN: split-file %s %t

// RUN: %clang_cc1 -fsyntax-only -fmodules -fmodules-cache-path=%t -fmodule-map-file=%t/module.modulemap %t/test.cppm -I%t
//

//--- test.cppm
#pragma clang module import Baz

//--- Foo.h
#pragma once
class foo {
char dummy = 1;

public:
static foo var;

};

inline foo foo::var;

//--- Bar.h
#pragma once
#include <Foo.h>

void bar() {
(void) foo::var;
}

//--- Baz.h
#pragma once
#include <Foo.h>

void baz() {
(void) foo::var;
}

#include <Bar.h>

//--- module.modulemap
module Foo {
header "Foo.h"
}
module Bar {
header "Bar.h"
}
module Baz {
header "Baz.h"
}