[clang] Allow ObjC writeback conversion in cleanup attribute type check (PR #195318)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Akira Hatanaka (ahatanak)

<details>
<summary>Changes</summary>

Prior to #<!-- -->164440, CheckAssignmentConstraints in the cleanup attribute handler ran before ObjC lifetime qualifiers were inferred on the variable. It compared against a type without '__strong' and accepted both 'T **' to 'T *__autoreleasing *' and 'T **' to 'T *__unsafe_unretained *'. #<!-- -->164440 reversed the order, so the check now runs after '__strong' is inferred and rejects both 'T *__strong *' to 'T *__autoreleasing *' and 'T *__strong *' to
'T *__unsafe_unretained *'.

Fix the valid case by falling back to isObjCWritebackConversion when the assignment check fails. This re-allows the '__strong' to '__autoreleasing' writeback conversion while continuing to reject '__strong' to '__unsafe_unretained'.

rdar://175133715

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


2 Files Affected:

- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+4-2) 
- (modified) clang/test/SemaObjC/attr-cleanup.m (+28-2) 


``````````diff
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 76073ed80175f..99111e5e9027c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -8851,8 +8851,10 @@ void Sema::ActOnCleanupAttr(Decl *D, const Attr *A) {
   // If this ever proves to be a problem it should be easy to fix.
   QualType Ty = this->Context.getPointerType(VD->getType());
   QualType ParamTy = FD->getParamDecl(0)->getType();
-  if (!this->IsAssignConvertCompatible(this->CheckAssignmentConstraints(
-          FD->getParamDecl(0)->getLocation(), ParamTy, Ty))) {
+  if (QualType ConvertedTy;
+      !this->IsAssignConvertCompatible(this->CheckAssignmentConstraints(
+          FD->getParamDecl(0)->getLocation(), ParamTy, Ty)) &&
+      !ObjC().isObjCWritebackConversion(Ty, ParamTy, ConvertedTy)) {
     this->Diag(Attr->getArgLoc(),
                diag::err_attribute_cleanup_func_arg_incompatible_type)
         << NI.getName() << ParamTy << Ty;
diff --git a/clang/test/SemaObjC/attr-cleanup.m b/clang/test/SemaObjC/attr-cleanup.m
index 9ee02bfab5a2a..9666041806e14 100644
--- a/clang/test/SemaObjC/attr-cleanup.m
+++ b/clang/test/SemaObjC/attr-cleanup.m
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only
-// expected-no-diagnostics
+// RUN: %clang_cc1 %s -verify=noarc -fsyntax-only -fblocks
+// RUN: %clang_cc1 %s -verify=arc -fsyntax-only -fblocks -fobjc-arc
+// noarc-no-diagnostics
 
 @class NSString;
 
@@ -9,3 +10,28 @@ void t1(void)
 {
   NSString *s __attribute((cleanup(c1)));
 }
+
+// Cleanup function with __autoreleasing parameter should be accepted under ARC
+// via the writeback conversion (*__strong * -> *__autoreleasing *).
+void c2(NSString *__autoreleasing *obj) { (void)obj; }
+
+void t2(void) {
+  NSString *x __attribute__((cleanup(c2))) = (void *)0;
+}
+
+typedef void (^DeferBlock)(void);
+void c3(DeferBlock __autoreleasing *block) {
+  if (block && *block) (*block)();
+}
+
+void t3(void) {
+  DeferBlock x __attribute__((cleanup(c3))) = ^{};
+}
+
+// Writeback conversion doesn't apply to __unsafe_unretained parameters;
+// only __autoreleasing is valid.
+void c4(NSString *__unsafe_unretained *obj) { (void)obj; }
+
+void t4(void) {
+  NSString *x __attribute__((cleanup(c4))) = (void *)0; // arc-error {{'cleanup' function 'c4' parameter has type 'NSString *__unsafe_unretained *' which is incompatible with type 'NSString *__strong *'}}
+}

``````````

</details>


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