Author: Kyungtak Woo Date: 2026-07-09T12:58:50-06:00 New Revision: 5114f5e4269bd7b189b1500889cd88d0d1eff033
URL: https://github.com/llvm/llvm-project/commit/5114f5e4269bd7b189b1500889cd88d0d1eff033 DIFF: https://github.com/llvm/llvm-project/commit/5114f5e4269bd7b189b1500889cd88d0d1eff033.diff LOG: [Clang][Sema] Ensure explicitly defaulted functions respect FP pragmas from their declaration site (#207429) Currently, when an explicitly defaulted function is synthesized (e.g., at its first use), it erroneously adopts the floating-point (FP) pragma state of the synthesis site rather than its declaration site. This leads to mismatched or incorrect FP features being applied to the generated body, completely ignoring `#pragma STDC FENV_ACCESS` pragmas that were active when the function was explicitly defaulted. This change should fix this issue by capturing and restoring the FP features at the appropriate times. Fixes #207266 I also added the following fixes to support the above: - AST Importer Fix: Updated ASTImporter::VisitFunctionDecl to correctly import FPFeatures and Lookups for defaulted/deleted functions, whereas before it was silently dropping them. - AST Version Bump: Bumped VERSION_MAJOR to 39 in ASTBitCodes.h to reflect the new FunctionDecl binary layout changes in the AST serialization. Added: clang/test/CodeGenCXX/defaulted-function-fp-features.cpp Modified: clang/include/clang/AST/Decl.h clang/include/clang/Basic/LangOptions.h clang/include/clang/Serialization/ASTBitCodes.h clang/lib/AST/ASTImporter.cpp clang/lib/AST/Decl.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaTemplateInstantiateDecl.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 88c900203630a..a6e9b3bcaa608 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2049,13 +2049,17 @@ class FunctionDecl : public DeclaratorDecl, }; - /// Stashed information about a defaulted/deleted function body. + /// Stashed information about a defaulted/deleted function body, including + /// the active FP pragma overrides (FPOptionsOverride) from the declaration + /// site. These overrides are required to correctly synthesize the function + /// body. class DefaultedOrDeletedFunctionInfo final : llvm::TrailingObjects<DefaultedOrDeletedFunctionInfo, DeclAccessPair, StringLiteral *> { friend TrailingObjects; unsigned NumLookups; bool HasDeletedMessage; + FPOptionsOverride FPFeatures; size_t numTrailingObjects(OverloadToken<DeclAccessPair>) const { return NumLookups; @@ -2064,8 +2068,11 @@ class FunctionDecl : public DeclaratorDecl, public: static DefaultedOrDeletedFunctionInfo * Create(ASTContext &Context, ArrayRef<DeclAccessPair> Lookups, + FPOptionsOverride FPFeatures, StringLiteral *DeletedMessage = nullptr); + FPOptionsOverride getFPFeatures() const { return FPFeatures; } + /// Get the unqualified lookup results that should be used in this /// defaulted function definition. ArrayRef<DeclAccessPair> getUnqualifiedLookups() const { diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 6c86300fc5581..f21131622d03d 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -989,6 +989,10 @@ class FPOptions { /// /// The is implemented as a value of the new FPOptions plus a mask showing which /// fields are actually set in it. +/// +/// NOTE: This type is serialized into the AST format (e.g. for defaulted +/// functions). When adding a new field here or in FPOptions, ensure that the +/// AST VERSION_MAJOR is bumped if it changes the layout or size. class FPOptionsOverride { FPOptions Options = FPOptions::getFromOpaqueInt(0); FPOptions::storage_type OverrideMask = 0; diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 279380de2f7fe..73388948b5750 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -44,7 +44,7 @@ namespace serialization { /// Version 4 of AST files also requires that the version control branch and /// revision match exactly, since there is no backward compatibility of /// AST files at this time. -const unsigned VERSION_MAJOR = 38; +const unsigned VERSION_MAJOR = 39; /// AST file minor version number supported by this version of /// Clang. diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 64e9b7aebd1d1..d1de2dbac2a0e 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -4246,13 +4246,7 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { // decl and its redeclarations may be required. } - StringLiteral *Msg = D->getDeletedMessage(); - if (Msg) { - auto Imported = import(Msg); - if (!Imported) - return Imported.takeError(); - Msg = *Imported; - } + // We will import DefaultedOrDeletedInfo later. ToFunction->setQualifierInfo(ToQualifierLoc); ToFunction->setAccess(D->getAccess()); @@ -4271,10 +4265,28 @@ ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) { ToFunction->setRangeEnd(ToEndLoc); ToFunction->setDefaultLoc(ToDefaultLoc); - if (Msg) + if (auto *Info = D->getDefaultedOrDeletedInfo()) { + StringLiteral *Msg = nullptr; + if (StringLiteral *M = Info->getDeletedMessage()) { + auto Imported = import(M); + if (!Imported) + return Imported.takeError(); + Msg = *Imported; + } + + SmallVector<DeclAccessPair, 4> Lookups; + for (DeclAccessPair P : Info->getUnqualifiedLookups()) { + auto Imported = import(P.getDecl()); + if (!Imported) + return Imported.takeError(); + Lookups.push_back( + DeclAccessPair::make(cast<NamedDecl>(*Imported), P.getAccess())); + } + ToFunction->setDefaultedOrDeletedInfo( FunctionDecl::DefaultedOrDeletedFunctionInfo::Create( - Importer.getToContext(), {}, Msg)); + Importer.getToContext(), Lookups, Info->getFPFeatures(), Msg)); + } // Set the parameters. for (auto *Param : Parameters) { diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index b23bf73ae803c..4eaef0d87f3e5 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3116,7 +3116,7 @@ bool FunctionDecl::isVariadic() const { FunctionDecl::DefaultedOrDeletedFunctionInfo * FunctionDecl::DefaultedOrDeletedFunctionInfo::Create( ASTContext &Context, ArrayRef<DeclAccessPair> Lookups, - StringLiteral *DeletedMessage) { + FPOptionsOverride FPFeatures, StringLiteral *DeletedMessage) { static constexpr size_t Alignment = std::max({alignof(DefaultedOrDeletedFunctionInfo), alignof(DeclAccessPair), alignof(StringLiteral *)}); @@ -3127,6 +3127,7 @@ FunctionDecl::DefaultedOrDeletedFunctionInfo::Create( new (Context.Allocate(Size, Alignment)) DefaultedOrDeletedFunctionInfo; Info->NumLookups = Lookups.size(); Info->HasDeletedMessage = DeletedMessage != nullptr; + Info->FPFeatures = FPFeatures; llvm::uninitialized_copy(Lookups, Info->getTrailingObjects<DeclAccessPair>()); if (DeletedMessage) @@ -3152,7 +3153,7 @@ void FunctionDecl::setDeletedAsWritten(bool D, StringLiteral *Message) { DefaultedOrDeletedInfo->setDeletedMessage(Message); else setDefaultedOrDeletedInfo(DefaultedOrDeletedFunctionInfo::Create( - getASTContext(), /*Lookups=*/{}, Message)); + getASTContext(), /*Lookups=*/{}, FPOptionsOverride(), Message)); } } diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 65b46562b166e..15c3b04942825 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -6906,6 +6906,25 @@ Sema::getDefaultedFunctionKind(const FunctionDecl *FD) { return DefaultedFunctionKind(); } +namespace { +/// RAII object to restore the floating-point (FP) features active at the time +/// a defaulted function was declared. This ensures that the synthesized body +/// of the function respects the FP pragmas (e.g., #pragma STDC FENV_ACCESS) +/// that were in effect when the function was explicitly defaulted. +struct DefaultedFunctionFPFeaturesRAII { + Sema::FPFeaturesStateRAII SavedFPFeatures; + DefaultedFunctionFPFeaturesRAII(Sema &S, FunctionDecl *FD) + : SavedFPFeatures(S) { + auto *Info = FD->getDefaultedOrDeletedInfo(); + FPOptionsOverride FPO = Info ? Info->getFPFeatures() : FPOptionsOverride(); + S.CurFPFeatures = FPO.applyOverrides(S.LangOpts); + S.FpPragmaStack.CurrentValue = FPO; + } + + ~DefaultedFunctionFPFeaturesRAII() = default; +}; +} // namespace + static void DefineDefaultedFunction(Sema &S, FunctionDecl *FD, SourceLocation DefaultLoc) { Sema::DefaultedFunctionKind DFK = S.getDefaultedFunctionKind(FD); @@ -9029,7 +9048,7 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD, FD->getOverloadedOperator()); FD->setDefaultedOrDeletedInfo( FunctionDecl::DefaultedOrDeletedFunctionInfo::Create( - Context, Operators.pairs())); + Context, Operators.pairs(), CurFPFeatureOverrides())); } // C++2a [class.compare.default]p1: @@ -9374,6 +9393,8 @@ void Sema::DefineDefaultedComparison(SourceLocation UseLoc, FunctionDecl *FD, // Add a context note for diagnostics produced after this point. Scope.addContextNote(UseLoc); + DefaultedFunctionFPFeaturesRAII RestoreFP(*this, FD); + { // Build and set up the function body. // The first parameter has type maybe-ref-to maybe-const T, use that to get @@ -14367,6 +14388,7 @@ CXXConstructorDecl *Sema::DeclareImplicitDefaultConstructor( void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation, CXXConstructorDecl *Constructor) { + DefaultedFunctionFPFeaturesRAII RestoreFP(*this, Constructor); assert((Constructor->isDefaulted() && Constructor->isDefaultConstructor() && !Constructor->doesThisDeclarationHaveABody() && !Constructor->isDeleted()) && @@ -14666,6 +14688,7 @@ CXXDestructorDecl *Sema::DeclareImplicitDestructor(CXXRecordDecl *ClassDecl) { void Sema::DefineImplicitDestructor(SourceLocation CurrentLocation, CXXDestructorDecl *Destructor) { + DefaultedFunctionFPFeaturesRAII RestoreFP(*this, Destructor); assert((Destructor->isDefaulted() && !Destructor->doesThisDeclarationHaveABody() && !Destructor->isDeleted()) && @@ -15356,6 +15379,7 @@ static void diagnoseDeprecatedCopyOperation(Sema &S, CXXMethodDecl *CopyOp) { void Sema::DefineImplicitCopyAssignment(SourceLocation CurrentLocation, CXXMethodDecl *CopyAssignOperator) { + DefaultedFunctionFPFeaturesRAII RestoreFP(*this, CopyAssignOperator); assert((CopyAssignOperator->isDefaulted() && CopyAssignOperator->isOverloadedOperator() && CopyAssignOperator->getOverloadedOperator() == OO_Equal && @@ -15743,6 +15767,7 @@ static void checkMoveAssignmentForRepeatedMove(Sema &S, CXXRecordDecl *Class, void Sema::DefineImplicitMoveAssignment(SourceLocation CurrentLocation, CXXMethodDecl *MoveAssignOperator) { + DefaultedFunctionFPFeaturesRAII RestoreFP(*this, MoveAssignOperator); assert((MoveAssignOperator->isDefaulted() && MoveAssignOperator->isOverloadedOperator() && MoveAssignOperator->getOverloadedOperator() == OO_Equal && @@ -16076,6 +16101,7 @@ CXXConstructorDecl *Sema::DeclareImplicitCopyConstructor( void Sema::DefineImplicitCopyConstructor(SourceLocation CurrentLocation, CXXConstructorDecl *CopyConstructor) { + DefaultedFunctionFPFeaturesRAII RestoreFP(*this, CopyConstructor); assert((CopyConstructor->isDefaulted() && CopyConstructor->isCopyConstructor() && !CopyConstructor->doesThisDeclarationHaveABody() && @@ -16214,6 +16240,7 @@ CXXConstructorDecl *Sema::DeclareImplicitMoveConstructor( void Sema::DefineImplicitMoveConstructor(SourceLocation CurrentLocation, CXXConstructorDecl *MoveConstructor) { + DefaultedFunctionFPFeaturesRAII RestoreFP(*this, MoveConstructor); assert((MoveConstructor->isDefaulted() && MoveConstructor->isMoveConstructor() && !MoveConstructor->doesThisDeclarationHaveABody() && @@ -18776,6 +18803,16 @@ void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) { return; } + // Only allocate DefaultedOrDeletedFunctionInfo if we actually have + // non-default FP features to stash. This avoids memory overhead for + // the vast majority of defaulted functions. + if (!FD->getDefaultedOrDeletedInfo() && + CurFPFeatureOverrides().requiresTrailingStorage()) { + FD->setDefaultedOrDeletedInfo( + FunctionDecl::DefaultedOrDeletedFunctionInfo::Create( + Context, /*Lookups=*/{}, CurFPFeatureOverrides())); + } + if (DefKind.isComparison()) { if (CheckExplicitlyDefaultedComparison(nullptr, FD, DefKind.asComparison())) FD->setInvalidDecl(); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index c56203f10ac3c..a787d537f02f8 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -5515,11 +5515,10 @@ bool TemplateDeclInstantiator::SubstDefaultedFunction(FunctionDecl *New, Lookups.push_back(DeclAccessPair::make(D, DA.getAccess())); } - // It's unlikely that substitution will change any declarations. Don't - // store an unnecessary copy in that case. New->setDefaultedOrDeletedInfo( AnyChanged ? FunctionDecl::DefaultedOrDeletedFunctionInfo::Create( - SemaRef.Context, Lookups) + SemaRef.Context, Lookups, DFI->getFPFeatures(), + DFI->getDeletedMessage()) : DFI); } diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index bd684b89e194f..f484bc7e0bf55 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1094,6 +1094,9 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) { StringLiteral *DeletedMessage = HasMessage ? cast<StringLiteral>(Record.readExpr()) : nullptr; + FPOptionsOverride FPFeatures = + FPOptionsOverride::getFromOpaqueInt(Record.readInt()); + unsigned NumLookups = Record.readInt(); SmallVector<DeclAccessPair, 8> Lookups; for (unsigned I = 0; I != NumLookups; ++I) { @@ -1104,7 +1107,7 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) { FD->setDefaultedOrDeletedInfo( FunctionDecl::DefaultedOrDeletedFunctionInfo::Create( - Reader.getContext(), Lookups, DeletedMessage)); + Reader.getContext(), Lookups, FPFeatures, DeletedMessage)); } } diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index f271769d8edf6..e3b81650d65a1 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -898,6 +898,8 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { if (DeletedMessage) Record.AddStmt(DeletedMessage); + Record.push_back(FDI->getFPFeatures().getAsOpaqueInt()); + Record.push_back(FDI->getUnqualifiedLookups().size()); for (DeclAccessPair P : FDI->getUnqualifiedLookups()) { Record.AddDeclRef(P.getDecl()); diff --git a/clang/test/CodeGenCXX/defaulted-function-fp-features.cpp b/clang/test/CodeGenCXX/defaulted-function-fp-features.cpp new file mode 100644 index 0000000000000..809c1fc864618 --- /dev/null +++ b/clang/test/CodeGenCXX/defaulted-function-fp-features.cpp @@ -0,0 +1,101 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-linux -std=c++20 -emit-llvm -o - %s | FileCheck %s + +// CHECK-DAG: define {{.*}} @_ZeqRK6TargetS1_({{.*}}) [[NORMAL_ATTRS:#[0-9]+]] +// CHECK-DAG: define {{.*}} @_ZeqRK12StrictTargetS1_({{.*}}) [[STRICT_ATTRS:#[0-9]+]] +// CHECK-DAG: define {{.*}} @_ZN12AssignTargetaSERKS_({{.*}}) [[NORMAL_ATTRS]] +// CHECK-DAG: define {{.*}} @_ZN18StrictAssignTargetaSERKS_({{.*}}) [[STRICT_ATTRS]] + +// Templates +// CHECK-DAG: define {{.*}} @_ZeqRK14TemplateTargetIdES2_({{.*}}) [[NORMAL_ATTRS]] +// CHECK-DAG: define {{.*}} @_ZeqRK14TemplateTargetIfES2_({{.*}}) [[NORMAL_ATTRS]] +// CHECK-DAG: define {{.*}} @_ZeqRK20StrictTemplateTargetIdES2_({{.*}}) [[STRICT_ATTRS]] + +// --- NON-STRICT DECLARATIONS (at top of file, default FP is non-strict) --- + +struct Target { + double d; + friend bool operator==(const Target&, const Target&) = default; +}; + +struct Member { + double d; + Member& operator=(const Member& Other) { + d = Other.d + 1.0; + return *this; + } +}; + +struct AssignTarget { + Member m; +}; + +template <typename T> +struct TemplateTarget { + T d; + friend bool operator==(const TemplateTarget&, const TemplateTarget&) = default; +}; + +// Trigger instantiation of TemplateTarget<double>::operator== in non-strict context. +bool test_template_non_strict(TemplateTarget<double> a, TemplateTarget<double> b) { + return a == b; +} + + +// --- STRICT CONTEXT (pragmas enabled) --- +#pragma STDC FENV_ACCESS ON + +// Use-sites triggering synthesis of non-strict defaulted functions in strict context. + +bool test_non_strict_cmp(Target a, Target b) { + return a == b; +} + +void test_non_strict_assign(AssignTarget& a, AssignTarget& b) { + a = b; +} + +// Trigger instantiation of TemplateTarget<float>::operator== in strict context. +// It should still be non-strict because the template was defined in non-strict context. +bool test_template_strict(TemplateTarget<float> a, TemplateTarget<float> b) { + return a == b; +} + +// Strict declarations (must get strictfp) + +struct StrictTarget { + double d; + friend bool operator==(const StrictTarget&, const StrictTarget&) = default; +}; + +bool test_strict_cmp(StrictTarget a, StrictTarget b) { + return a == b; +} + +struct StrictAssignTarget { + Member m; +}; + +void test_strict_assign(StrictAssignTarget& a, StrictAssignTarget& b) { + a = b; +} + +template <typename T> +struct StrictTemplateTarget { + T d; + friend bool operator==(const StrictTemplateTarget&, const StrictTemplateTarget&) = default; +}; + + +// --- NON-STRICT CONTEXT AGAIN --- +#pragma STDC FENV_ACCESS OFF + +// Trigger instantiation of StrictTemplateTarget<double>::operator== in non-strict context. +// It should be strict because the template was defined in strict context. +bool test_strict_template_non_strict(StrictTemplateTarget<double> a, StrictTemplateTarget<double> b) { + return a == b; +} + + +// CHECK-DAG: attributes [[STRICT_ATTRS]] = { {{.*}}strictfp{{.*}} } +// CHECK-DAG: attributes [[NORMAL_ATTRS]] = { {{.*}} } +// CHECK-NOT: attributes [[NORMAL_ATTRS]] = { {{.*}}strictfp _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
