[clang] 'unused-but-set-variable' warning should not apply to __attribute__((objc_precise_lifetime) Objective-C pointers

[mwyman]

Summary

The objc_precise_lifetime attribute is applied to Objective-C pointers to ensure the optimizer does not prematurely release an object under Automatic Reference Counting (ARC). It is a common enough pattern to assign values to these variables but not reference them otherwise, and annotating them with __unused is not really correct as they are being used to ensure an object's lifetime.

[stephanemoore]

Make sense to me.

I don't know offhand who might be a good second pair of eyes to take a look.

clang/test/SemaObjC/objc-precise-lifetime-unused-variable.m

6–8

Is it worth adding some cases with nested scopes? I think that's a common scenario where objc_precise_lifetime is often used?

[stephanemoore]

Maybe ahatanak might be a good reviewer based on https://reviews.llvm.org/D112850?

[dmaclach]

clang/test/SemaObjC/objc-precise-lifetime-unused-variable.m

7

what happens in the case that x is never assigned to anything? Do we still get a warning?

[mwyman]

clang/test/SemaObjC/objc-precise-lifetime-unused-variable.m

7

The code implementing -Wunused-variable checking already explicitly excludes objc_precise_lifetime-attributed variables: https://github.com/llvm/llvm-project/blob/36e335eeb577b6dc559de3a66bc832afae1f56c4/clang/lib/Sema/SemaDecl.cpp#L1847

[mwyman]

Added new inner scope for test file.

clang/lib/Sema/SemaDecl.cpp

void Sema::DiagnoseUnusedButSetDecl(const VarDecl *VD) {
   }
 
   // Don't warn about __block Objective-C pointer variables, as they might
   // be assigned in the block but not used elsewhere for the purpose of lifetime
   // extension.
   if (VD->hasAttr<BlocksAttr>() && Ty->isObjCObjectPointerType())
     return;
 
+  // Don't warn about Objective-C pointer variables with precise lifetime
+  // semantics; they can be used to ensure ARC releases the object at a known
+  // time, which may mean assignment but no other references.
+  if (VD->hasAttr<ObjCPreciseLifetimeAttr>() && Ty->isObjCObjectPointerType())
+    return;
+
   auto iter = RefsMinusAssignments.find(VD);
   if (iter == RefsMinusAssignments.end())
     return;
 
   assert(iter->getSecond() >= 0 &&
          "Found a negative number of references to a VarDecl");
   if (iter->getSecond() != 0)
     return;

clang/test/SemaObjC/objc-precise-lifetime-unused-variable.m

+// RUN: %clang_cc1 -triple x86_64-apple-macos11 -fsyntax-only -fobjc-arc -fblocks -verify -Wunused-but-set-variable -Wno-objc-root-class %s
+
+id getFoo(void);
+
+void test() {
+  // no diagnostics for objects with precise lifetime semantics.
+  __attribute__((objc_precise_lifetime)) id x;
+  x = getFoo();
+
+  id x2; // expected-warning {{variable 'x2' set but not used}}
+  x2 = getFoo();
+}