[clang] [Clang] Fix cleanup attribute by delaying type checks after the type is deduced (PR #164440)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Guillot Tony (to268)

<details>
<summary>Changes</summary>

Previously, the handling of the `cleanup` attribute had some checks based on the type, but we were deducing the type after handling the attribute.
This PR fixes the way the are dealing with type checks for the `cleanup` attribute by delaying these checks after we are deducing the type.

It is also fixed in a way that the solution can be adapted for other attributes that does some type based checks.
This is the list of C/C++ attributes that are doing type based checks and will need to be fixed in additional PRs:
- CUDAShared
- MutualExclusions
- PassObjectSize
- InitPriority
- Sentinel
- AcquireCapability
- RequiresCapability
- LocksExcluded
- AcquireHandle

NB: Some attributes could have been missed in my shallow search.

Fixes #<!-- -->129631

---
Full diff: https://github.com/llvm/llvm-project/pull/164440.diff


5 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+1) 
- (modified) clang/include/clang/Sema/Sema.h (+7) 
- (modified) clang/lib/Sema/SemaDecl.cpp (+18) 
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+24-10) 
- (added) clang/test/Sema/type-dependent-attrs.c (+11) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fe77f917bb801..491cfd3351861 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -447,6 +447,7 @@ Bug Fixes to Attribute Support
 - Using ``[[gnu::cleanup(some_func)]]`` where some_func is annotated with
   ``[[gnu::error("some error")]]`` now correctly triggers an error. (#GH146520)
 - Fix a crash when the function name is empty in the `swift_name` attribute. (#GH157075)
+- 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 cb21335ede075..09fb20aa54a6f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -4465,6 +4465,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
@@ -15483,6 +15487,9 @@ 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 fc3aabf5741ca..13600218b9d83 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3354,6 +3354,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,
@@ -13797,6 +13812,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);
@@ -14587,6 +14604,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 9475b8a684082..a60ad17ed1eb0 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.
@@ -8291,3 +8281,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 *'}}
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/164440