Author: Henrik G. Olsson Date: 2025-06-23T10:16:31-07:00 New Revision: 319a51a5ffb807b88ae7f73676894bf306a0d134
URL: https://github.com/llvm/llvm-project/commit/319a51a5ffb807b88ae7f73676894bf306a0d134 DIFF: https://github.com/llvm/llvm-project/commit/319a51a5ffb807b88ae7f73676894bf306a0d134.diff LOG: [Modules] Record whether VarDecl initializers contain side effects (#143739) Calling `DeclMustBeEmitted` should not lead to more deserialization, as it may occur before previous deserialization has finished. When passed a `VarDecl` with an initializer however, `DeclMustBeEmitted` needs to know whether that initializer contains side effects. When the `VarDecl` is deserialized but the initializer is not, this triggers deserialization of the initializer. To avoid this we add a bit to the serialization format for `VarDecl`s, indicating whether its initializer contains side effects or not, so that the `ASTReader` can query this information directly without deserializing the initializer. rdar://153085264 Added: clang/test/Modules/var-init-side-effects.cpp Modified: clang/include/clang/AST/Decl.h clang/include/clang/AST/ExternalASTSource.h clang/include/clang/Sema/MultiplexExternalSemaSource.h clang/include/clang/Serialization/ASTReader.h clang/lib/AST/ASTContext.cpp clang/lib/AST/Decl.cpp clang/lib/Sema/MultiplexExternalSemaSource.cpp clang/lib/Serialization/ASTReader.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/lib/Serialization/ASTWriterDecl.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 05aac15b30cd6..23cd25e2e3071 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -1352,6 +1352,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. + 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 f45e3af7602c1..e91d5132da10f 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. /// @@ -195,6 +196,10 @@ 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. /// @@ -429,6 +434,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 391c2177d75ec..7c66c26a17a13 100644 --- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h +++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h @@ -94,6 +94,8 @@ 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, diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index be1c6e0817593..7a4b7d21bb20e 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1455,6 +1455,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<Decl *, 16> InitSideEffectVars; + public: /// Get the buffer for resolving paths. SmallString<0> &getPathBuf() { return PathBuf; } @@ -2406,6 +2412,8 @@ 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); diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 02d6570d0ea0f..faad0427178b4 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -13111,9 +13111,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 20a3db1c69b81..9e993f9291a7a 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -2441,6 +2441,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<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; +} + bool VarDecl::isOutOfLine() const { if (Decl::isOutOfLine()) return true; diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp index fbfb242598c24..9f19f13592e86 100644 --- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp +++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp @@ -115,6 +115,14 @@ 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) { diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index a3fbc3d25acab..6f082fe840b4c 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9722,6 +9722,10 @@ 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)); } diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 7f7882654b9d1..0ffd78424be0d 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1628,6 +1628,9 @@ 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 2d93832a9ac31..2e390dbe79ec6 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -1306,6 +1306,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(); @@ -1355,10 +1356,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<ImplicitParamDecl>(D) && !D->isEscapingByref()) + !D->hasInitWithSideEffects() && !D->isEscapingByref() && + !HasDeducedType && D->getStorageDuration() != SD_Static && + !D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() && + !D->isObjCForDecl() && !isa<ImplicitParamDecl>(D) && + !D->isEscapingByref()) AbbrevToUse = Writer.getDeclVarAbbrev(); Code = serialization::DECL_VAR; @@ -2731,12 +2733,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; +} + _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
