[CIR][LifetimeCheck] Integrate temporary check into checkPointerDeref

Refactor use-after-move detection per feedback from PR #2077:
- Make checkPointerDeref return bool indicating error status
- Integrate isSkippableTemporary check into checkPointerDeref
- Add temporary checks to marking functions (markOwnerAsMovedFrom,
  markPointerOrValueTypeAsMovedFrom)
- Simplify call sites in checkArgForRValueRef
- Fix emittedDiagnostics guard placement to preserve moved-from tracking
- Preserve original behavior: pointer/value types only check Invalid state

This reduces duplicated guard patterns and consolidates temporary
handling in checkPointerDeref.

Includes regression test for multi-arg rvalue-ref scenarios.

Author: ivanmurashko

clang/lib/CIR/Dialect/Transforms/LifetimeCheck.cpp

@@ -70,7 +70,7 @@ struct LifetimeCheckPass : public LifetimeCheckBase<LifetimeCheckPass> {
     CallParam,
     IndirectCallParam,
   };
-  void checkPointerDeref(mlir::Value addr, mlir::Location loc,
+  bool checkPointerDeref(mlir::Value addr, mlir::Location loc,
                          DerefStyle derefStyle = DerefStyle::Direct);
   void checkCoroTaskStore(StoreOp storeOp);
   void checkLambdaCaptureStore(StoreOp storeOp);
@@ -1515,8 +1515,12 @@ void LifetimeCheckPass::emitInvalidHistory(mlir::InFlightDiagnostic &D,
   }
 }
 
-void LifetimeCheckPass::checkPointerDeref(mlir::Value addr, mlir::Location loc,
+bool LifetimeCheckPass::checkPointerDeref(mlir::Value addr, mlir::Location loc,
                                           DerefStyle derefStyle) {
+  // Skip temporaries early - they're about to be destroyed anyway
+  if (isSkippableTemporary(addr))
+    return false; // Not an error, just a temporary
+
   bool hasInvalid = getPmap()[addr].count(State::getInvalid());
   bool hasNullptr = getPmap()[addr].count(State::getNullPtr());
 
@@ -1527,10 +1531,6 @@ void LifetimeCheckPass::checkPointerDeref(mlir::Value addr, mlir::Location loc,
     emitRemark(loc) << "pset => " << Out.str();
   };
 
-  // Do not emit the same warning twice or more.
-  if (emittedDiagnostics.count(loc))
-    return;
-
   bool psetRemarkEmitted = false;
   if (opts.emitRemarkPsetAlways()) {
     emitPsetRemark();
@@ -1539,15 +1539,20 @@ void LifetimeCheckPass::checkPointerDeref(mlir::Value addr, mlir::Location loc,
 
   // 2.4.2 - On every dereference of a Pointer p, enforce that p is valid.
   if (!hasInvalid && !hasNullptr)
-    return;
+    return false; // No error
 
   // TODO: create verbosity/accuracy levels, for now use deref styles directly
   // to decide when not to emit a warning.
 
   // For indirect calls, do not relly on blunt nullptr passing, require some
   // invalidation to have happened in a path.
   if (derefStyle == DerefStyle::IndirectCallParam && !hasInvalid)
-    return;
+    return false; // No error
+
+  // Do not emit the same warning twice or more (check AFTER we know there's an
+  // error)
+  if (emittedDiagnostics.count(loc))
+    return true; // Already reported
 
   // Ok, filtered out questionable warnings, take the bad path leading to this
   // deference point and diagnose it.
@@ -1588,6 +1593,8 @@ void LifetimeCheckPass::checkPointerDeref(mlir::Value addr, mlir::Location loc,
 
   if (!psetRemarkEmitted && opts.emitRemarkPsetInvalid())
     emitPsetRemark();
+
+  return true; // Error was reported
 }
 
 static FuncOp getCalleeFromSymbol(ModuleOp mod, llvm::StringRef name) {
@@ -1727,35 +1734,28 @@ void LifetimeCheckPass::checkArgForRValueRef(
 
   // Owner types
   if (owners.count(addr)) {
-    if (getPmap()[addr].count(State::getInvalid()) ||
-        getPmap()[addr].count(State::getNullPtr())) {
-      checkPointerDeref(addr, callOp.getLoc());
-      return;
-    }
-    if (!isSkippableTemporary(addr))
-      markOwnerAsMovedFrom(addr, callOp.getLoc());
+    if (checkPointerDeref(addr, callOp.getLoc()))
+      return; // Already reported error
+    markOwnerAsMovedFrom(addr, callOp.getLoc());
     return;
   }
 
-  // Pointer types
+  // Pointer types - only check Invalid (not NullPtr)
   if (ptrs.count(addr)) {
     if (getPmap()[addr].count(State::getInvalid())) {
       checkPointerDeref(addr, callOp.getLoc());
       return;
     }
-    if (!isSkippableTemporary(addr))
-      markPointerOrValueTypeAsMovedFrom(addr, callOp.getLoc());
+    markPointerOrValueTypeAsMovedFrom(addr, callOp.getLoc());
     return;
   }
 
-  // Value types (primitives)
+  // Value types - only check Invalid (not NullPtr)
   if (getPmap()[addr].count(State::getInvalid())) {
     checkPointerDeref(addr, callOp.getLoc());
     return;
   }
-
-  if (!isSkippableTemporary(addr))
-    markPointerOrValueTypeAsMovedFrom(addr, callOp.getLoc());
+  markPointerOrValueTypeAsMovedFrom(addr, callOp.getLoc());
 }
 
 void LifetimeCheckPass::checkCopyAssignment(CallOp callOp,
@@ -1916,6 +1916,10 @@ void LifetimeCheckPass::checkNonConstUseOfOwner(mlir::Value ownerAddr,
 
 void LifetimeCheckPass::markOwnerAsMovedFrom(mlir::Value addr,
                                              mlir::Location loc) {
+  // Don't mark temporaries as moved-from - they're about to be destroyed
+  if (isSkippableTemporary(addr))
+    return;
+
   // All owner types are marked invalid after move. Safe operations on specific
   // owner types (e.g., smart pointer get/reset) are handled separately in
   // isSmartPointerSafeMethod.
@@ -1936,6 +1940,10 @@ bool LifetimeCheckPass::isValueTypeMovedFrom(mlir::Value addr) {
 
 void LifetimeCheckPass::markPointerOrValueTypeAsMovedFrom(mlir::Value addr,
                                                           mlir::Location loc) {
+  // Don't mark temporaries as moved-from
+  if (isSkippableTemporary(addr))
+    return;
+
   markPsetInvalid(addr, InvalidStyle::MovedFrom, loc);
 }
 
@@ -1984,18 +1992,14 @@ void LifetimeCheckPass::checkMoveCtor(CallOp callOp, cir::CXXCtorAttr ctor) {
   // Check if this is an Owner type move constructor
   auto addr = getThisParamOwnerCategory(callOp);
   if (addr && owners.count(src)) {
-    // Don't mark temporaries as moved-from - they're about to be destroyed
-    if (!isSkippableTemporary(src))
-      markOwnerAsMovedFrom(src, callOp.getLoc());
+    markOwnerAsMovedFrom(src, callOp.getLoc());
     return;
   }
 
   // Check if this is a Pointer type move constructor
   addr = getThisParamPointerCategory(callOp);
   if (addr && ptrs.count(src)) {
-    // Don't mark temporaries as moved-from
-    if (!isSkippableTemporary(src))
-      markPointerOrValueTypeAsMovedFrom(src, callOp.getLoc());
+    markPointerOrValueTypeAsMovedFrom(src, callOp.getLoc());
     return;
   }
 }
@@ -2183,7 +2187,7 @@ void LifetimeCheckPass::checkCall(CallOp callOp) {
   // invalid access to Ptr if any of its methods are used.
   auto addr = getThisParamPointerCategory(callOp);
   if (addr)
-    return checkPointerDeref(addr, callOp.getLoc());
+    checkPointerDeref(addr, callOp.getLoc());
 }
 
 void LifetimeCheckPass::checkOperation(Operation *op) {

clang/test/CIR/Transforms/lifetime-check-smart-pointer-after-move.cpp

@@ -166,6 +166,7 @@ void test_shared_ptr_arrow_after_move() {
 
 // Test 8: Move via function parameter (unique_ptr)
 void consume_unique_ptr(std::unique_ptr<int>&& ptr) {}
+void consume_two_unique_ptrs(std::unique_ptr<int>&& ptr1, std::unique_ptr<int>&& ptr2) {}
 
 void test_unique_ptr_move_via_param() {
   std::unique_ptr<int> p(new int(42));
@@ -225,3 +226,14 @@ void test_release_after_move() {
 
   int* raw = p.release(); // OK - release() is safe
 }
+
+// Test 14: Multiple owner arguments with rvalue references
+// Regression test for emittedDiagnostics guard bug
+void test_multi_arg_owner_move() {
+  std::unique_ptr<int> x(new int(1));
+  std::unique_ptr<int> y(new int(2));
+  consume_unique_ptr(std::move(x)); // expected-note {{moved here via std::move or rvalue reference}}
+  consume_two_unique_ptrs(std::move(x), std::move(y)); // expected-warning {{use of invalid pointer 'x'}}
+                                                        // expected-note@-1 {{moved here via std::move or rvalue reference}}
+  int use_y = *y; // expected-warning {{use of invalid pointer 'y'}}
+}