https://github.com/to268 updated https://github.com/llvm/llvm-project/pull/164440
>From dbb1bf61aadf4e1f19925606a82accc747639868 Mon Sep 17 00:00:00 2001 From: Guillot Tony <[email protected]> Date: Tue, 21 Oct 2025 15:21:33 +0200 Subject: [PATCH 1/8] Fix for cleaup attribute and sets base for fixing other type dependent attributes Fixes #129631 --- clang/docs/ReleaseNotes.rst | 1 + clang/include/clang/Sema/Sema.h | 6 +++++ clang/lib/Sema/SemaDecl.cpp | 18 ++++++++++++++ clang/lib/Sema/SemaDeclAttr.cpp | 34 ++++++++++++++++++-------- clang/test/Sema/type-dependent-attrs.c | 11 +++++++++ 5 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 clang/test/Sema/type-dependent-attrs.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7459127670cc3..8d984ded13c77 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -500,6 +500,7 @@ Bug Fixes to Attribute Support - Fixes crashes or missing diagnostics with the `device_kernel` attribute. (#GH161905) - Fix handling of parameter indexes when an attribute is applied to a C++23 explicit object member function. - Fixed several false positives and false negatives in function effect (`nonblocking`) analysis. (#GH166078) (#GH166101) (#GH166110) +- Fix ``cleanup`` attribute by delaying type checks after the type is deduced. (#GH129631) Bug Fixes to C++ Support ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 6ca182338d6af..d848f9929e41d 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4456,6 +4456,10 @@ class Sema final : public SemaBase { NamedDecl *New, Decl *Old, AvailabilityMergeKind AMK = AvailabilityMergeKind::Redeclaration); + /// CheckAttributesOnDeducedType - Calls Sema functions for attributes that + /// requires the type to be deduced. + void CheckAttributesOnDeducedType(Expr *E, Decl *D); + /// MergeTypedefNameDecl - We just parsed a typedef 'New' which has the /// same name and scope as a previous declaration 'Old'. Figure out /// how to resolve this situation, merging decls or emitting @@ -15469,6 +15473,8 @@ class Sema final : public SemaBase { std::optional<FunctionEffectMode> ActOnEffectExpression(Expr *CondExpr, StringRef AttributeName); + void ActOnCleanupAttr(Expr *E, Decl *D, const Attr *A); + private: /// The implementation of RequireCompleteType bool RequireCompleteTypeImpl(SourceLocation Loc, QualType T, diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 25b89d65847ad..9a7787a44f9ae 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -3355,6 +3355,21 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old, if (!foundAny) New->dropAttrs(); } +void Sema::CheckAttributesOnDeducedType(Expr *E, Decl *D) { + if (!D->hasAttrs()) + return; + + for (const Attr *A : D->getAttrs()) { + switch (A->getKind()) { + case attr::Cleanup: + ActOnCleanupAttr(E, D, A); + break; + default: + continue; + } + } +} + // Returns the number of added attributes. template <class T> static unsigned propagateAttribute(ParmVarDecl *To, const ParmVarDecl *From, @@ -13809,6 +13824,8 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) { return; } + this->CheckAttributesOnDeducedType(Init, RealDecl); + // dllimport cannot be used on variable definitions. if (VDecl->hasAttr<DLLImportAttr>() && !VDecl->isStaticDataMember()) { Diag(VDecl->getLocation(), diag::err_attribute_dllimport_data_definition); @@ -14599,6 +14616,7 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) { Var->setInit(RecoveryExpr.get()); } + this->CheckAttributesOnDeducedType(Init.get(), RealDecl); CheckCompleteVariableDeclaration(Var); } } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index a9e7b44ac9d73..2a6c163f624e8 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3511,16 +3511,6 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) { return; } - // We're currently more strict than GCC about what function types we accept. - // If this ever proves to be a problem it should be easy to fix. - QualType Ty = S.Context.getPointerType(cast<VarDecl>(D)->getType()); - QualType ParamTy = FD->getParamDecl(0)->getType(); - if (!S.IsAssignConvertCompatible(S.CheckAssignmentConstraints( - FD->getParamDecl(0)->getLocation(), ParamTy, Ty))) { - S.Diag(Loc, diag::err_attribute_cleanup_func_arg_incompatible_type) - << NI.getName() << ParamTy << Ty; - return; - } VarDecl *VD = cast<VarDecl>(D); // Create a reference to the variable declaration. This is a fake/dummy // reference. @@ -8311,3 +8301,27 @@ void Sema::redelayDiagnostics(DelayedDiagnosticPool &pool) { assert(curPool && "re-emitting in undelayed context not supported"); curPool->steal(pool); } + +void Sema::ActOnCleanupAttr(Expr *E, Decl *D, const Attr *A) { + FunctionDecl *FD = nullptr; + DeclarationNameInfo NI; + CleanupAttr *Attr = D->getAttr<CleanupAttr>(); + + // Obtains the FunctionDecl that was found when handling the attribute + // earlier. + FD = Attr->getFunctionDecl(); + NI = FD->getNameInfo(); + + // We're currently more strict than GCC about what function types we accept. + // If this ever proves to be a problem it should be easy to fix. + QualType Ty = this->Context.getPointerType(cast<VarDecl>(D)->getType()); + QualType ParamTy = FD->getParamDecl(0)->getType(); + if (!this->IsAssignConvertCompatible(this->CheckAssignmentConstraints( + FD->getParamDecl(0)->getLocation(), ParamTy, Ty))) { + this->Diag(Attr->getArgLoc(), + diag::err_attribute_cleanup_func_arg_incompatible_type) + << NI.getName() << ParamTy << Ty; + D->dropAttr<CleanupAttr>(); + return; + } +} diff --git a/clang/test/Sema/type-dependent-attrs.c b/clang/test/Sema/type-dependent-attrs.c new file mode 100644 index 0000000000000..f99293d1cd639 --- /dev/null +++ b/clang/test/Sema/type-dependent-attrs.c @@ -0,0 +1,11 @@ +// RUN: %clang_cc1 -std=c23 -fsyntax-only -verify %s + +// #GH129631-CleanupAttr +int open() { return 0; } +void close(typeof(open()) *) {} + +void cleanup_attr() { + int fd_int [[gnu::cleanup(close)]] = open(); + auto fd_auto [[gnu::cleanup(close)]] = open(); + float fd_invalid [[gnu::cleanup(close)]] = open(); // expected-error {{'cleanup' function 'close' parameter has type 'typeof (open()) *' (aka 'int *') which is incompatible with type 'float *'}} +} >From 47b1841bd87d4d6810b40c37fd865e24fb97492b Mon Sep 17 00:00:00 2001 From: Guillot Tony <[email protected]> Date: Tue, 21 Oct 2025 17:40:44 +0200 Subject: [PATCH 2/8] Fixed small issues that where appointed --- clang/lib/Sema/SemaDeclAttr.cpp | 9 +++------ clang/test/Sema/type-dependent-attrs.c | 1 - 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 2a6c163f624e8..65bfeb9ddbebe 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -8303,14 +8303,11 @@ void Sema::redelayDiagnostics(DelayedDiagnosticPool &pool) { } void Sema::ActOnCleanupAttr(Expr *E, Decl *D, const Attr *A) { - FunctionDecl *FD = nullptr; - DeclarationNameInfo NI; - CleanupAttr *Attr = D->getAttr<CleanupAttr>(); - // Obtains the FunctionDecl that was found when handling the attribute // earlier. - FD = Attr->getFunctionDecl(); - NI = FD->getNameInfo(); + CleanupAttr *Attr = D->getAttr<CleanupAttr>(); + FunctionDecl *FD = Attr->getFunctionDecl(); + DeclarationNameInfo NI = FD->getNameInfo(); // We're currently more strict than GCC about what function types we accept. // If this ever proves to be a problem it should be easy to fix. diff --git a/clang/test/Sema/type-dependent-attrs.c b/clang/test/Sema/type-dependent-attrs.c index f99293d1cd639..13068b3f94ad4 100644 --- a/clang/test/Sema/type-dependent-attrs.c +++ b/clang/test/Sema/type-dependent-attrs.c @@ -1,6 +1,5 @@ // RUN: %clang_cc1 -std=c23 -fsyntax-only -verify %s -// #GH129631-CleanupAttr int open() { return 0; } void close(typeof(open()) *) {} >From 92134329c5fdc9ca79cba7671f295b61a188585b Mon Sep 17 00:00:00 2001 From: Guillot Tony <[email protected]> Date: Fri, 24 Oct 2025 11:56:31 +0200 Subject: [PATCH 3/8] Reworked switch to be generated using TableGen --- clang/include/clang/Basic/Attr.td | 4 ++++ clang/include/clang/Sema/CMakeLists.txt | 5 +++++ clang/include/clang/Sema/Sema.h | 2 ++ clang/lib/Sema/SemaDecl.cpp | 8 +------ clang/utils/TableGen/ClangAttrEmitter.cpp | 26 +++++++++++++++++++++++ clang/utils/TableGen/TableGen.cpp | 7 ++++++ clang/utils/TableGen/TableGenBackends.h | 2 ++ llvm/docs/TableGen/BackEnds.rst | 7 ++++++ 8 files changed, 54 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 8dfe4bc08c48e..5f28613bb5a7e 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -741,6 +741,9 @@ class Attr { // our existing general parsing we need to have a separate flag that // opts an attribute into strict parsing of attribute parameters bit StrictEnumParameters = 0; + // Set to true for attributes which have Sema checks which requires the type + // to be deduced. + bit IsTypeDependent = 0; // Lists language options, one of which is required to be true for the // attribute to be applicable. If empty, no language options are required. list<LangOpt> LangOpts = []; @@ -1400,6 +1403,7 @@ def Cleanup : InheritableAttr { let Args = [DeclArgument<Function, "FunctionDecl">]; let Subjects = SubjectList<[LocalVar]>; let Documentation = [CleanupDocs]; + bit IsTypeDependent = 1; // FIXME: DeclArgument should be reworked to also store the // Expr instead of adding attr specific hacks like the following. // See the discussion in https://github.com/llvm/llvm-project/pull/14023. diff --git a/clang/include/clang/Sema/CMakeLists.txt b/clang/include/clang/Sema/CMakeLists.txt index 9077e22c2307c..3f540ea596871 100644 --- a/clang/include/clang/Sema/CMakeLists.txt +++ b/clang/include/clang/Sema/CMakeLists.txt @@ -8,6 +8,11 @@ clang_tablegen(AttrParsedAttrKinds.inc -gen-clang-attr-parsed-attr-kinds SOURCE ../Basic/Attr.td TARGET ClangAttrParsedAttrKinds) +clang_tablegen(AttrIsTypeDependent.inc -gen-clang-attr-is-type-dependent + -I ${CMAKE_CURRENT_SOURCE_DIR}/../../ + SOURCE ../Basic/Attr.td + TARGET ClangAttrIsTypeDependent) + clang_tablegen(AttrSpellingListIndex.inc -gen-clang-attr-spelling-index -I ${CMAKE_CURRENT_SOURCE_DIR}/../../ SOURCE ../Basic/Attr.td diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index d848f9929e41d..2820dd1030c33 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4764,6 +4764,8 @@ class Sema final : public SemaBase { // linkage or not. static bool mightHaveNonExternalLinkage(const DeclaratorDecl *FD); +#include "clang/Sema/AttrIsTypeDependent.inc" + ///@} // diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 9a7787a44f9ae..1fc9a43f36dfb 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -3360,13 +3360,7 @@ void Sema::CheckAttributesOnDeducedType(Expr *E, Decl *D) { return; for (const Attr *A : D->getAttrs()) { - switch (A->getKind()) { - case attr::Cleanup: - ActOnCleanupAttr(E, D, A); - break; - default: - continue; - } + checkAttrIsTypeDependent(E, D, A); } } diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index e49dcb9b70b0f..76ae33f5edf4b 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp @@ -5045,6 +5045,32 @@ void EmitClangAttrParsedAttrKinds(const RecordKeeper &Records, << "}\n"; } +// Emits Sema calls for type dependent attributes +void EmitClangAttrIsTypeDependent(const RecordKeeper &Records, + raw_ostream &OS) { + emitSourceFileHeader("Attribute is type dependent", OS, Records); + + std::set<StringRef> Seen; + for (const auto *A : Records.getAllDerivedDefinitions("Attr")) { + const Record &Attr = *A; + if (Attr.getValueAsBit("IsTypeDependent")) { + Seen.insert(Attr.getName()); + } + } + + OS << "void checkAttrIsTypeDependent(Expr *E, Decl *D, const Attr *A) {\n"; + OS << " switch (A->getKind()) {\n"; + for (const StringRef &SeenAttr : Seen) { + OS << " case attr::" << SeenAttr << ":\n"; + OS << " ActOn" << SeenAttr << "Attr(E, D, A);\n"; + OS << " break;\n"; + } + OS << " default:\n"; + OS << " break;\n"; + OS << " }\n"; + OS << "}\n"; +} + // Emits the code to dump an attribute. void EmitClangAttrTextNodeDump(const RecordKeeper &Records, raw_ostream &OS) { emitSourceFileHeader("Attribute text node dumper", OS, Records); diff --git a/clang/utils/TableGen/TableGen.cpp b/clang/utils/TableGen/TableGen.cpp index 866040d503646..707ce617cb2d0 100644 --- a/clang/utils/TableGen/TableGen.cpp +++ b/clang/utils/TableGen/TableGen.cpp @@ -43,6 +43,7 @@ enum ActionType { GenClangAttrParsedAttrList, GenClangAttrParsedAttrImpl, GenClangAttrParsedAttrKinds, + GenClangAttrIsTypeDependent, GenClangAttrTextNodeDump, GenClangAttrNodeTraverse, GenClangBasicReader, @@ -179,6 +180,9 @@ cl::opt<ActionType> Action( clEnumValN(GenClangAttrParsedAttrKinds, "gen-clang-attr-parsed-attr-kinds", "Generate a clang parsed attribute kinds"), + clEnumValN(GenClangAttrIsTypeDependent, + "gen-clang-attr-is-type-dependent", + "Generate clang is type dependent attribute code"), clEnumValN(GenClangAttrTextNodeDump, "gen-clang-attr-text-node-dump", "Generate clang attribute text node dumper"), clEnumValN(GenClangAttrNodeTraverse, "gen-clang-attr-node-traverse", @@ -423,6 +427,9 @@ bool ClangTableGenMain(raw_ostream &OS, const RecordKeeper &Records) { case GenClangAttrParsedAttrKinds: EmitClangAttrParsedAttrKinds(Records, OS); break; + case GenClangAttrIsTypeDependent: + EmitClangAttrIsTypeDependent(Records, OS); + break; case GenClangAttrTextNodeDump: EmitClangAttrTextNodeDump(Records, OS); break; diff --git a/clang/utils/TableGen/TableGenBackends.h b/clang/utils/TableGen/TableGenBackends.h index fa49dcd289bc2..058bda3ebd246 100644 --- a/clang/utils/TableGen/TableGenBackends.h +++ b/clang/utils/TableGen/TableGenBackends.h @@ -82,6 +82,8 @@ void EmitClangAttrParsedAttrImpl(const llvm::RecordKeeper &Records, llvm::raw_ostream &OS); void EmitClangAttrParsedAttrKinds(const llvm::RecordKeeper &Records, llvm::raw_ostream &OS); +void EmitClangAttrIsTypeDependent(const llvm::RecordKeeper &Records, + llvm::raw_ostream &OS); void EmitClangAttrTextNodeDump(const llvm::RecordKeeper &Records, llvm::raw_ostream &OS); void EmitClangAttrNodeTraverse(const llvm::RecordKeeper &Records, diff --git a/llvm/docs/TableGen/BackEnds.rst b/llvm/docs/TableGen/BackEnds.rst index 7f571378860b2..1e3cb8783df16 100644 --- a/llvm/docs/TableGen/BackEnds.rst +++ b/llvm/docs/TableGen/BackEnds.rst @@ -355,6 +355,13 @@ ClangAttrParsedAttrKinds ``AttributeList::getKind`` function, mapping a string (and syntax) to a parsed attribute ``AttributeList::Kind`` enumeration. +ClangAttrIsTypeDependent +------------------------ + +**Purpose**: Creates ``AttrIsTypeDependent.inc``, which is used to implement the +``Sema::CheckAttributesOnDeducedType`` function, mapping an attribute kind to a +Sema function if it exists. + ClangAttrDump ------------- >From c384d3a9cd82e53643663a17ca3c9af8f17abd01 Mon Sep 17 00:00:00 2001 From: Guillot Tony <[email protected]> Date: Fri, 24 Oct 2025 16:00:26 +0200 Subject: [PATCH 4/8] Simplified EmitClangAttrIsTypeDependent --- clang/utils/TableGen/ClangAttrEmitter.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index 76ae33f5edf4b..0036b45f029e8 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp @@ -5050,20 +5050,14 @@ void EmitClangAttrIsTypeDependent(const RecordKeeper &Records, raw_ostream &OS) { emitSourceFileHeader("Attribute is type dependent", OS, Records); - std::set<StringRef> Seen; - for (const auto *A : Records.getAllDerivedDefinitions("Attr")) { - const Record &Attr = *A; - if (Attr.getValueAsBit("IsTypeDependent")) { - Seen.insert(Attr.getName()); - } - } - OS << "void checkAttrIsTypeDependent(Expr *E, Decl *D, const Attr *A) {\n"; OS << " switch (A->getKind()) {\n"; - for (const StringRef &SeenAttr : Seen) { - OS << " case attr::" << SeenAttr << ":\n"; - OS << " ActOn" << SeenAttr << "Attr(E, D, A);\n"; - OS << " break;\n"; + for (const auto *A : Records.getAllDerivedDefinitions("Attr")) { + if (A->getValueAsBit("IsTypeDependent")) { + OS << " case attr::" << A->getName() << ":\n"; + OS << " ActOn" << A->getName() << "Attr(E, D, A);\n"; + OS << " break;\n"; + } } OS << " default:\n"; OS << " break;\n"; >From 8830c895885e3c604b04a2f15fe20c70c684ebbc Mon Sep 17 00:00:00 2001 From: Guillot Tony <[email protected]> Date: Sat, 25 Oct 2025 15:53:54 +0200 Subject: [PATCH 5/8] Removed unused parameter and added comment for IsTypeDependent field --- clang/include/clang/Basic/Attr.td | 8 ++++++++ clang/include/clang/Sema/Sema.h | 4 ++-- clang/lib/Sema/SemaDecl.cpp | 9 +++++---- clang/lib/Sema/SemaDeclAttr.cpp | 2 +- clang/utils/TableGen/ClangAttrEmitter.cpp | 4 ++-- 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 5f28613bb5a7e..8e1707b65ae86 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -743,6 +743,14 @@ class Attr { bit StrictEnumParameters = 0; // Set to true for attributes which have Sema checks which requires the type // to be deduced. + // When `IsTypeDependent` is set to true, you should add an `ActOn*Attr` + // function to `Sema.h`. The signature of the function must be: + // `void ActOn*Attr(Decl *, const Attr *);` where the `Decl *` is the + // declaration the attribute will be attached to; its type will have already + // been deduced, and the `Attr *` is the attribute being applied to that + // declaration. This function should handle all type-sensitive semantics for + // the attribute. This function will be automatically called by + // `Sema::CheckAttributesOnDeducedType()`. bit IsTypeDependent = 0; // Lists language options, one of which is required to be true for the // attribute to be applicable. If empty, no language options are required. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 2820dd1030c33..fd2a2469142e4 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -4458,7 +4458,7 @@ class Sema final : public SemaBase { /// CheckAttributesOnDeducedType - Calls Sema functions for attributes that /// requires the type to be deduced. - void CheckAttributesOnDeducedType(Expr *E, Decl *D); + void CheckAttributesOnDeducedType(Decl *D); /// MergeTypedefNameDecl - We just parsed a typedef 'New' which has the /// same name and scope as a previous declaration 'Old'. Figure out @@ -15475,7 +15475,7 @@ class Sema final : public SemaBase { std::optional<FunctionEffectMode> ActOnEffectExpression(Expr *CondExpr, StringRef AttributeName); - void ActOnCleanupAttr(Expr *E, Decl *D, const Attr *A); + void ActOnCleanupAttr(Decl *D, const Attr *A); private: /// The implementation of RequireCompleteType diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 1fc9a43f36dfb..ca5c936a6464d 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -3355,12 +3355,12 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old, if (!foundAny) New->dropAttrs(); } -void Sema::CheckAttributesOnDeducedType(Expr *E, Decl *D) { +void Sema::CheckAttributesOnDeducedType(Decl *D) { if (!D->hasAttrs()) return; for (const Attr *A : D->getAttrs()) { - checkAttrIsTypeDependent(E, D, A); + checkAttrIsTypeDependent(D, A); } } @@ -13818,7 +13818,7 @@ void Sema::AddInitializerToDecl(Decl *RealDecl, Expr *Init, bool DirectInit) { return; } - this->CheckAttributesOnDeducedType(Init, RealDecl); + this->CheckAttributesOnDeducedType(RealDecl); // dllimport cannot be used on variable definitions. if (VDecl->hasAttr<DLLImportAttr>() && !VDecl->isStaticDataMember()) { @@ -14311,6 +14311,8 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) { DeduceVariableDeclarationType(Var, false, nullptr)) return; + this->CheckAttributesOnDeducedType(RealDecl); + // C++11 [class.static.data]p3: A static data member can be declared with // the constexpr specifier; if so, its declaration shall specify // a brace-or-equal-initializer. @@ -14610,7 +14612,6 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl) { Var->setInit(RecoveryExpr.get()); } - this->CheckAttributesOnDeducedType(Init.get(), RealDecl); CheckCompleteVariableDeclaration(Var); } } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 65bfeb9ddbebe..00ea62f712a61 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -8302,7 +8302,7 @@ void Sema::redelayDiagnostics(DelayedDiagnosticPool &pool) { curPool->steal(pool); } -void Sema::ActOnCleanupAttr(Expr *E, Decl *D, const Attr *A) { +void Sema::ActOnCleanupAttr(Decl *D, const Attr *A) { // Obtains the FunctionDecl that was found when handling the attribute // earlier. CleanupAttr *Attr = D->getAttr<CleanupAttr>(); diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index 0036b45f029e8..db2a83d8cf578 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp @@ -5050,12 +5050,12 @@ void EmitClangAttrIsTypeDependent(const RecordKeeper &Records, raw_ostream &OS) { emitSourceFileHeader("Attribute is type dependent", OS, Records); - OS << "void checkAttrIsTypeDependent(Expr *E, Decl *D, const Attr *A) {\n"; + OS << "void checkAttrIsTypeDependent(Decl *D, const Attr *A) {\n"; OS << " switch (A->getKind()) {\n"; for (const auto *A : Records.getAllDerivedDefinitions("Attr")) { if (A->getValueAsBit("IsTypeDependent")) { OS << " case attr::" << A->getName() << ":\n"; - OS << " ActOn" << A->getName() << "Attr(E, D, A);\n"; + OS << " ActOn" << A->getName() << "Attr(D, A);\n"; OS << " break;\n"; } } >From b5d1d6b418188837b74773936992e19979d2d2a4 Mon Sep 17 00:00:00 2001 From: Guillot Tony <[email protected]> Date: Thu, 13 Nov 2025 17:59:15 +0100 Subject: [PATCH 6/8] Added handling of templated arguments and fixed missing ArgLoc --- clang/include/clang/Basic/Attr.td | 2 +- clang/lib/Sema/SemaDeclAttr.cpp | 5 +++- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 9 +++++++ clang/test/SemaCXX/attr-cleanup.cpp | 24 +++++++++++++++++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 8e1707b65ae86..0097476bc0d8d 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1411,7 +1411,7 @@ def Cleanup : InheritableAttr { let Args = [DeclArgument<Function, "FunctionDecl">]; let Subjects = SubjectList<[LocalVar]>; let Documentation = [CleanupDocs]; - bit IsTypeDependent = 1; + let IsTypeDependent = 1; // FIXME: DeclArgument should be reworked to also store the // Expr instead of adding attr specific hacks like the following. // See the discussion in https://github.com/llvm/llvm-project/pull/14023. diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 00ea62f712a61..84001b414549a 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -8308,10 +8308,13 @@ void Sema::ActOnCleanupAttr(Decl *D, const Attr *A) { CleanupAttr *Attr = D->getAttr<CleanupAttr>(); FunctionDecl *FD = Attr->getFunctionDecl(); DeclarationNameInfo NI = FD->getNameInfo(); + VarDecl *VD = cast<VarDecl>(D); + if (VD->getType()->isDependentType()) + return; // We're currently more strict than GCC about what function types we accept. // If this ever proves to be a problem it should be easy to fix. - QualType Ty = this->Context.getPointerType(cast<VarDecl>(D)->getType()); + QualType Ty = this->Context.getPointerType(VD->getType()); QualType ParamTy = FD->getParamDecl(0)->getType(); if (!this->IsAssignConvertCompatible(this->CheckAssignmentConstraints( FD->getParamDecl(0)->getLocation(), ParamTy, Ty))) { diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 1b6b559c1227b..3a4b2ccc74350 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1007,6 +1007,15 @@ void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs, continue; } + if (auto *A = dyn_cast<CleanupAttr>(TmplAttr)) { + if (!New->hasAttr<CleanupAttr>()) { + auto *NewAttr = A->clone(Context); + NewAttr->setArgLoc(A->getArgLoc()); + New->addAttr(NewAttr); + } + continue; + } + assert(!TmplAttr->isPackExpansion()); if (TmplAttr->isLateParsed() && LateAttrs) { // Late parsed attributes must be instantiated and attached after the diff --git a/clang/test/SemaCXX/attr-cleanup.cpp b/clang/test/SemaCXX/attr-cleanup.cpp index 32d10683edebb..3e7302e5362bf 100644 --- a/clang/test/SemaCXX/attr-cleanup.cpp +++ b/clang/test/SemaCXX/attr-cleanup.cpp @@ -27,3 +27,27 @@ namespace E { int v1 __attribute__((cleanup(c3))); // expected-error {{'c3' is not a single function}} } } + +namespace F { + int open() { return 0; } + void close(decltype(open()) *) {} + + void test1() { + auto fd [[gnu::cleanup(close)]] = open(); + } + + template <typename Ty> + void test2() { + Ty fd [[gnu::cleanup(close)]] = open(); + } + + template <typename Ty> + void test3() { + Ty fd [[gnu::cleanup(close)]] = open(); // expected-error {{'cleanup' function 'close' parameter has type 'decltype(open()) *' (aka 'int *') which is incompatible with type 'float *'}} + } + + int main() { + test2<int>(); + test3<float>(); // expected-note {{in instantiation of function template specialization 'F::test3<float>' requested here}} + } +} >From 3e8f013b11b9538a437262910617d537a60142a8 Mon Sep 17 00:00:00 2001 From: Guillot Tony <[email protected]> Date: Thu, 13 Nov 2025 19:44:29 +0100 Subject: [PATCH 7/8] Applied suggestions --- clang/lib/Sema/SemaDecl.cpp | 6 +----- clang/test/SemaCXX/attr-cleanup.cpp | 5 +++-- clang/utils/TableGen/ClangAttrEmitter.cpp | 4 ++-- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index ca5c936a6464d..b7aecadc86871 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -3356,12 +3356,8 @@ void Sema::mergeDeclAttributes(NamedDecl *New, Decl *Old, } void Sema::CheckAttributesOnDeducedType(Decl *D) { - if (!D->hasAttrs()) - return; - - for (const Attr *A : D->getAttrs()) { + for (const Attr *A : D->attrs()) checkAttrIsTypeDependent(D, A); - } } // Returns the number of added attributes. diff --git a/clang/test/SemaCXX/attr-cleanup.cpp b/clang/test/SemaCXX/attr-cleanup.cpp index 3e7302e5362bf..6048b4e92ec3f 100644 --- a/clang/test/SemaCXX/attr-cleanup.cpp +++ b/clang/test/SemaCXX/attr-cleanup.cpp @@ -43,11 +43,12 @@ namespace F { template <typename Ty> void test3() { - Ty fd [[gnu::cleanup(close)]] = open(); // expected-error {{'cleanup' function 'close' parameter has type 'decltype(open()) *' (aka 'int *') which is incompatible with type 'float *'}} + Ty fd [[gnu::cleanup(close)]] = open(); // #TEST3_CLEANUP } int main() { test2<int>(); - test3<float>(); // expected-note {{in instantiation of function template specialization 'F::test3<float>' requested here}} + test3<float>(); // expected-error@#TEST3_CLEANUP {{'cleanup' function 'close' parameter has type 'decltype(open()) *' (aka 'int *') which is incompatible with type 'float *'}} \ + expected-note {{in instantiation of function template specialization 'F::test3<float>' requested here}} } } diff --git a/clang/utils/TableGen/ClangAttrEmitter.cpp b/clang/utils/TableGen/ClangAttrEmitter.cpp index db2a83d8cf578..bee9a01a3b01a 100644 --- a/clang/utils/TableGen/ClangAttrEmitter.cpp +++ b/clang/utils/TableGen/ClangAttrEmitter.cpp @@ -5052,6 +5052,8 @@ void EmitClangAttrIsTypeDependent(const RecordKeeper &Records, OS << "void checkAttrIsTypeDependent(Decl *D, const Attr *A) {\n"; OS << " switch (A->getKind()) {\n"; + OS << " default:\n"; + OS << " break;\n"; for (const auto *A : Records.getAllDerivedDefinitions("Attr")) { if (A->getValueAsBit("IsTypeDependent")) { OS << " case attr::" << A->getName() << ":\n"; @@ -5059,8 +5061,6 @@ void EmitClangAttrIsTypeDependent(const RecordKeeper &Records, OS << " break;\n"; } } - OS << " default:\n"; - OS << " break;\n"; OS << " }\n"; OS << "}\n"; } >From d70e7c290c38ab75aade4cb2ed6334a69f374c55 Mon Sep 17 00:00:00 2001 From: Guillot Tony <[email protected]> Date: Mon, 17 Nov 2025 20:46:26 +0100 Subject: [PATCH 8/8] Fixed a few things from Aaron --- clang/docs/ReleaseNotes.rst | 2 +- clang/lib/Sema/SemaDeclAttr.cpp | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8d984ded13c77..c2da61e4d066a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -500,7 +500,7 @@ Bug Fixes to Attribute Support - Fixes crashes or missing diagnostics with the `device_kernel` attribute. (#GH161905) - Fix handling of parameter indexes when an attribute is applied to a C++23 explicit object member function. - Fixed several false positives and false negatives in function effect (`nonblocking`) analysis. (#GH166078) (#GH166101) (#GH166110) -- Fix ``cleanup`` attribute by delaying type checks after the type is deduced. (#GH129631) +- Fix ``cleanup`` attribute by delaying type checks until after the type is deduced. (#GH129631) Bug Fixes to C++ Support ^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 84001b414549a..bda7aa32a9348 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -8303,14 +8303,15 @@ void Sema::redelayDiagnostics(DelayedDiagnosticPool &pool) { } void Sema::ActOnCleanupAttr(Decl *D, const Attr *A) { + VarDecl *VD = cast<VarDecl>(D); + if (VD->getType()->isDependentType()) + return; + // Obtains the FunctionDecl that was found when handling the attribute // earlier. CleanupAttr *Attr = D->getAttr<CleanupAttr>(); FunctionDecl *FD = Attr->getFunctionDecl(); DeclarationNameInfo NI = FD->getNameInfo(); - VarDecl *VD = cast<VarDecl>(D); - if (VD->getType()->isDependentType()) - return; // We're currently more strict than GCC about what function types we accept. // If this ever proves to be a problem it should be easy to fix. _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
