From 4cd8b963fb28b9d14af2e3ec790051be75642a07 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 7 Jul 2025 16:06:29 -0700 Subject: [PATCH 1/3] Reland "Cherry-pick "[Modules] Record whether VarDecl initializers contain side effects (#143739)" This reverts commit db18f41b83ba42e4c1542c2abc158cae59361f78. --- clang/include/clang/AST/Decl.h | 5 +++ clang/include/clang/AST/ExternalASTSource.h | 16 ++++++++ .../clang/Sema/MultiplexExternalSemaSource.h | 2 + clang/include/clang/Serialization/ASTReader.h | 8 ++++ clang/lib/AST/ASTContext.cpp | 4 +- clang/lib/AST/Decl.cpp | 24 ++++++++++++ clang/lib/AST/ExprConstant.cpp | 6 +++ .../lib/Sema/MultiplexExternalSemaSource.cpp | 8 ++++ clang/lib/Serialization/ASTReader.cpp | 4 ++ clang/lib/Serialization/ASTReaderDecl.cpp | 3 ++ clang/lib/Serialization/ASTWriterDecl.cpp | 14 ++++--- clang/test/Modules/var-init-side-effects.cpp | 37 +++++++++++++++++++ 12 files changed, 122 insertions(+), 9 deletions(-) create mode 100644 clang/test/Modules/var-init-side-effects.cpp diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 50a712836e3dd..c601c02deb0c8 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -1339,6 +1339,11 @@ class VarDecl : public DeclaratorDecl, public Redeclarable { return const_cast(this)->getInitializingDeclaration(); } + /// Checks whether this declaration has an initializer with side effects, + /// without triggering deserialization if the initializer is not yet + /// deserialized. + bool hasInitWithSideEffects() const; + /// Determine whether this variable's value might be usable in a /// constant expression, according to the relevant language standard. /// This only checks properties of the declaration, and does not check diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h index 069a38cdc323f..7b09a4eb9414a 100644 --- a/clang/include/clang/AST/ExternalASTSource.h +++ b/clang/include/clang/AST/ExternalASTSource.h @@ -51,6 +51,7 @@ class RecordDecl; class Selector; class Stmt; class TagDecl; +class VarDecl; /// Abstract interface for external sources of AST nodes. /// @@ -173,6 +174,10 @@ class ExternalASTSource : public RefCountedBase { virtual ExtKind hasExternalDefinitions(const Decl *D); + virtual bool hasInitializerWithSideEffects(const VarDecl *VD) const { + return false; + } + /// Finds all declarations lexically contained within the given /// DeclContext, after applying an optional filter predicate. /// @@ -407,6 +412,17 @@ 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 { diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h index 238fb398b7d12..c7e6c4bc3ffe9 100644 --- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h +++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h @@ -92,6 +92,8 @@ class MultiplexExternalSemaSource : public ExternalSemaSource { ExtKind hasExternalDefinitions(const Decl *D) override; + bool hasInitializerWithSideEffects(const VarDecl *VD) const override; + /// Find all declarations with the given name in the /// given context. bool FindExternalVisibleDeclsByName(const DeclContext *DC, diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 51052290a20e6..90c71fc18fc30 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1348,6 +1348,12 @@ 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 InitSideEffectVars; + public: /// Get the buffer for resolving paths. SmallString<0> &getPathBuf() { return PathBuf; } @@ -2263,6 +2269,8 @@ class ASTReader ExtKind hasExternalDefinitions(const Decl *D) 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); diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 8ba7fa2479d04..973c2c84978c7 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -13262,9 +13262,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) { return true; // Variables that have initialization with side-effects are required. - if (VD->getInit() && VD->getInit()->HasSideEffects(*this) && - // We can get a value-dependent initializer during error recovery. - (VD->getInit()->isValueDependent() || !VD->evaluateValue())) + if (VD->hasInitWithSideEffects()) return true; // Likewise, variables with tuple-like bindings are required if their diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index cfbb3393183df..67a432d2a2cae 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -2440,6 +2440,30 @@ VarDecl *VarDecl::getInitializingDeclaration() { return Def; } +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(Init)) { + E = cast(S); + } else { + E = cast_or_null(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; +} + bool VarDecl::isOutOfLine() const { if (Decl::isOutOfLine()) return true; diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index d89b9ef6548b9..6e5be0eee74f6 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -16268,6 +16268,12 @@ static bool EvaluateInPlace(APValue &Result, EvalInfo &Info, const LValue &This, const Expr *E, bool AllowNonLiteralTypes) { assert(!E->isValueDependent()); + // Normally expressions passed to EvaluateInPlace have a type, but not when + // a VarDecl initializer is evaluated before the untyped ParenListExpr is + // replaced with a CXXConstructExpr. This can happen in LLDB. + if (E->getType().isNull()) + return false; + if (!AllowNonLiteralTypes && !CheckLiteralType(Info, E, &This)) return false; diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp index 79e656eb4b7e2..56be139814ce1 100644 --- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp +++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp @@ -107,6 +107,14 @@ MultiplexExternalSemaSource::hasExternalDefinitions(const Decl *D) { return EK_ReplyHazy; } +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) { bool AnyDeclsFound = false; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 591622f45394e..4da3d09bda3d2 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9268,6 +9268,10 @@ ExternalASTSource::ExtKind ASTReader::hasExternalDefinitions(const Decl *FD) { return I->second ? EK_Never : EK_Always; } +bool ASTReader::hasInitializerWithSideEffects(const VarDecl *VD) const { + return InitSideEffectVars.count(VD); +} + Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) { return DecodeSelector(getGlobalSelectorID(M, LocalID)); } diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 27dbc2f4c9218..7918cc88648e2 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1622,6 +1622,9 @@ ASTDeclReader::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 = diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index 41391cd0821d6..6885aef8031e7 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -1158,6 +1158,7 @@ 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(); @@ -1207,10 +1208,11 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) { !D->hasExtInfo() && D->getFirstDecl() == D->getMostRecentDecl() && D->getKind() == Decl::Var && !D->isInline() && !D->isConstexpr() && !D->isInitCapture() && !D->isPreviousDeclInSameBlockScope() && - !D->isEscapingByref() && !HasDeducedType && - D->getStorageDuration() != SD_Static && !D->getDescribedVarTemplate() && - !D->getMemberSpecializationInfo() && !D->isObjCForDecl() && - !isa(D) && !D->isEscapingByref()) + !D->hasInitWithSideEffects() && !D->isEscapingByref() && + !HasDeducedType && D->getStorageDuration() != SD_Static && + !D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() && + !D->isObjCForDecl() && !isa(D) && + !D->isEscapingByref()) AbbrevToUse = Writer.getDeclVarAbbrev(); Code = serialization::DECL_VAR; @@ -2529,12 +2531,12 @@ void ASTWriter::WriteDeclAbbrevs() { // VarDecl Abv->Add(BitCodeAbbrevOp( BitCodeAbbrevOp::Fixed, - 21)); // Packed Var Decl bits: Linkage, ModulesCodegen, + 22)); // Packed Var Decl bits: Linkage, ModulesCodegen, // SClass, TSCSpec, InitStyle, // isARCPseudoStrong, IsThisDeclarationADemotedDefinition, // isExceptionVariable, isNRVOVariable, isCXXForRangeDecl, // isInline, isInlineSpecified, isConstexpr, - // isInitCapture, isPrevDeclInSameScope, + // isInitCapture, isPrevDeclInSameScope, hasInitWithSideEffects, // EscapingByref, HasDeducedType, ImplicitParamKind, isObjCForDecl Abv->Add(BitCodeAbbrevOp(0)); // VarKind (local enum) // Type Source Info diff --git a/clang/test/Modules/var-init-side-effects.cpp b/clang/test/Modules/var-init-side-effects.cpp new file mode 100644 index 0000000000000..438a9eb204275 --- /dev/null +++ b/clang/test/Modules/var-init-side-effects.cpp @@ -0,0 +1,37 @@ +// Tests referencing variable with initializer containing side effect across module boundary +// +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/Foo.cppm \ +// RUN: -o %t/Foo.pcm + +// RUN: %clang_cc1 -std=c++20 -emit-module-interface \ +// RUN: -fmodule-file=Foo=%t/Foo.pcm \ +// RUN: %t/Bar.cppm \ +// RUN: -o %t/Bar.pcm + +// RUN: %clang_cc1 -std=c++20 -emit-obj \ +// RUN: -main-file-name Bar.cppm \ +// RUN: -fmodule-file=Foo=%t/Foo.pcm \ +// RUN: -x pcm %t/Bar.pcm \ +// RUN: -o %t/Bar.o + +//--- Foo.cppm +export module Foo; + +export { +class S {}; + +inline S s = S{}; +} + +//--- Bar.cppm +export module Bar; +import Foo; + +S bar() { + return s; +} + From 6aa28573060609d0dc00143fcb6ddc568165444f Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Thu, 3 Jul 2025 15:37:55 -0700 Subject: [PATCH 2/3] [Modules] Record side effect info in EvaluatedStmt (#146468) All deserialized VarDecl initializers are EvaluatedStmt, but not all EvaluatedStmt initializers are from a PCH. Calling `VarDecl::hasInitWithSideEffects` can trigger constant evaluation, but it's hard to know ahead of time whether that will trigger deserialization - even if the initializer is fully deserialized, it may contain a call to a constructor whose body is not deserialized. By caching the result of `VarDecl::hasInitWithSideEffects` and populating that cache during deserialization we can guarantee that calling it won't trigger deserialization regardless of the state of the initializer. This also reduces memory usage by removing the `InitSideEffectVars` set in `ASTReader`. rdar://154717930 (cherry picked from commit 2910c24638fcbc3dec02be072e6026d01012d946) --- clang/include/clang/AST/Decl.h | 14 +++-- clang/include/clang/AST/ExternalASTSource.h | 15 ------ .../clang/Sema/MultiplexExternalSemaSource.h | 2 - clang/include/clang/Serialization/ASTReader.h | 8 --- clang/lib/AST/Decl.cpp | 28 ++++------ .../lib/Sema/MultiplexExternalSemaSource.cpp | 8 --- clang/lib/Serialization/ASTReader.cpp | 4 -- clang/lib/Serialization/ASTReaderDecl.cpp | 5 +- clang/lib/Serialization/ASTWriter.cpp | 4 ++ clang/lib/Serialization/ASTWriterDecl.cpp | 1 - .../var-init-side-effects-modulemap.cpp | 51 +++++++++++++++++++ 11 files changed, 77 insertions(+), 63 deletions(-) create mode 100644 clang/test/Modules/var-init-side-effects-modulemap.cpp diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index c601c02deb0c8..04f5d2220dac9 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -874,13 +874,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. @@ -1339,9 +1343,11 @@ class VarDecl : public DeclaratorDecl, public Redeclarable { return const_cast(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 diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h index 7b09a4eb9414a..c201cc9933076 100644 --- a/clang/include/clang/AST/ExternalASTSource.h +++ b/clang/include/clang/AST/ExternalASTSource.h @@ -174,10 +174,6 @@ class ExternalASTSource : public RefCountedBase { virtual ExtKind hasExternalDefinitions(const Decl *D); - virtual bool hasInitializerWithSideEffects(const VarDecl *VD) const { - return false; - } - /// Finds all declarations lexically contained within the given /// DeclContext, after applying an optional filter predicate. /// @@ -412,17 +408,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 { diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h index c7e6c4bc3ffe9..238fb398b7d12 100644 --- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h +++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h @@ -92,8 +92,6 @@ class MultiplexExternalSemaSource : public ExternalSemaSource { ExtKind hasExternalDefinitions(const Decl *D) override; - bool hasInitializerWithSideEffects(const VarDecl *VD) const override; - /// Find all declarations with the given name in the /// given context. bool FindExternalVisibleDeclsByName(const DeclContext *DC, diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 90c71fc18fc30..51052290a20e6 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1348,12 +1348,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 InitSideEffectVars; - public: /// Get the buffer for resolving paths. SmallString<0> &getPathBuf() { return PathBuf; } @@ -2269,8 +2263,6 @@ class ASTReader ExtKind hasExternalDefinitions(const Decl *D) 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); diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 67a432d2a2cae..37528d73e92bc 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -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(Init)) { - E = cast(S); - } else { - E = cast_or_null(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 { diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp index 56be139814ce1..79e656eb4b7e2 100644 --- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp +++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp @@ -107,14 +107,6 @@ MultiplexExternalSemaSource::hasExternalDefinitions(const Decl *D) { return EK_ReplyHazy; } -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) { bool AnyDeclsFound = false; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 4da3d09bda3d2..591622f45394e 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9268,10 +9268,6 @@ ExternalASTSource::ExtKind ASTReader::hasExternalDefinitions(const Decl *FD) { return I->second ? EK_Never : EK_Always; } -bool ASTReader::hasInitializerWithSideEffects(const VarDecl *VD) const { - return InitSideEffectVars.count(VD); -} - Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) { return DecodeSelector(getGlobalSelectorID(M, LocalID)); } diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 7918cc88648e2..0ad3c2f7d4d14 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1622,9 +1622,6 @@ ASTDeclReader::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 = @@ -1695,6 +1692,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()) diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index dd1d35c15a60a..ccaf317b706f1 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -6834,6 +6834,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(); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index 6885aef8031e7..1ef19d206aada 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -1158,7 +1158,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(); diff --git a/clang/test/Modules/var-init-side-effects-modulemap.cpp b/clang/test/Modules/var-init-side-effects-modulemap.cpp new file mode 100644 index 0000000000000..750a6f1731405 --- /dev/null +++ b/clang/test/Modules/var-init-side-effects-modulemap.cpp @@ -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 + +void bar() { + (void) foo::var; +} + +//--- Baz.h +#pragma once +#include + +void baz() { + (void) foo::var; +} + +#include + +//--- module.modulemap +module Foo { + header "Foo.h" +} +module Bar { + header "Bar.h" +} +module Baz { + header "Baz.h" +} + From 668ed828a4181e81a370d7881e18aa89144590c2 Mon Sep 17 00:00:00 2001 From: "Henrik G. Olsson" Date: Mon, 7 Jul 2025 16:01:40 -0700 Subject: [PATCH 3/3] [Modules] Don't const eval VarDecls with dependent type (#147378) EvaluateAsInitializer does not support evaluating values with dependent types. This was previously guarded with a check for the initializer expression, but it is possible for the VarDecl to have a dependent type without the initializer having a dependent type, when the initializer is a specialized template type and the VarDecl has the unspecialized type. This adds a guard checking for dependence in the VarDecl type as well. This fixes the issue raised by Google in https://github.com/llvm/llvm-project/pull/145447 (cherry picked from commit c8850051c2414b899416e16222f5d96e854be563) --- clang/lib/AST/Decl.cpp | 3 ++- .../var-init-side-effects-templated.cpp | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 clang/test/Modules/var-init-side-effects-templated.cpp diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 37528d73e92bc..9e9916f5d7910 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -2450,7 +2450,8 @@ bool VarDecl::hasInitWithSideEffects() const { ES->HasSideEffects = E->HasSideEffects(getASTContext()) && // We can get a value-dependent initializer during error recovery. - (E->isValueDependent() || !evaluateValue()); + (E->isValueDependent() || getType()->isDependentType() || + !evaluateValue()); ES->CheckedForSideEffects = true; } return ES->HasSideEffects; diff --git a/clang/test/Modules/var-init-side-effects-templated.cpp b/clang/test/Modules/var-init-side-effects-templated.cpp new file mode 100644 index 0000000000000..542ca270429b6 --- /dev/null +++ b/clang/test/Modules/var-init-side-effects-templated.cpp @@ -0,0 +1,20 @@ +// Tests referencing variable with initializer containing side effect across module boundary + +// RUN: %clang_cc1 -std=c++20 -emit-module-interface %s -o %t + +export module Foo; + +export template +struct Wrapper { + double value; +}; + +export constexpr Wrapper Compute() { + return Wrapper{1.0}; +} + +export template +Wrapper ComputeInFloat() { + const Wrapper a = Compute(); + return a; +}