[-Wcalled-once-parameter] Let escapes overwrite MaybeCalled states

[vsavchenko]

Summary

This commit makes escapes symmetrical, meaning that having escape
before and after the branching, where parameter is not called on
one of the paths, will have the same effect.

[vsavchenko]

Unify the way we deal with escapes

[NoQ]

LGTM! Do any of the high-level comments at the beginning of the file need to be updated to reflect this change?

[vsavchenko]

In D98622#2630012, @NoQ wrote:

LGTM! Do any of the high-level comments at the beginning of the file need to be updated to reflect this change?

No, they don't. This commit doesn't affect the join semantics (the main focus of those). It changes semantics of the transfer function that probably also should be described somewhere in one part of the code.

clang/lib/Analysis/CalledOnceCheck.cpp

private:
   void checkIndirectCall(const CallLikeExpr *CallOrMessage) {
     // CallExpr::arguments does not interact nicely with llvm::enumerate.
     llvm::ArrayRef<const Expr *> Arguments = llvm::makeArrayRef(
         CallOrMessage->getArgs(), CallOrMessage->getNumArgs());
 
     // Let's check if any of the call arguments is a point of interest.
     for (const auto &Argument : llvm::enumerate(Arguments)) {
       if (auto Index = getIndexOfExpression(Argument.value())) {
-        ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index);
-
         if (shouldBeCalledOnce(CallOrMessage, Argument.index())) {
           // If the corresponding parameter is marked as 'called_once' we should
           // consider it as a call.
           processCallFor(*Index, CallOrMessage);
-        } else if (CurrentParamStatus.getKind() == ParameterStatus::NotCalled) {
+        } else {
           // Otherwise, we mark this parameter as escaped, which can be
           // interpreted both as called or not called depending on the context.
-          CurrentParamStatus = ParameterStatus::Escaped;
+          processEscapeFor(*Index);
         }
         // Otherwise, let's keep the state as it is.
       }
     }
   }
 
   /// Process call of the parameter with the given index
   void processCallFor(unsigned Index, const Expr *Call) {
void processCallFor(unsigned Index, const Expr *Call) {
     } else if (CurrentParamStatus.getKind() != ParameterStatus::Reported) {
       // If we didn't report anything yet, let's mark this parameter
       // as called.
       ParameterStatus Called(ParameterStatus::DefinitelyCalled, Call);
       CurrentParamStatus = Called;
     }
   }
 
+  /// Process escape of the parameter with the given index
+  void processEscapeFor(unsigned Index) {
+    ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(Index);
+
+    // Escape overrides whatever error we think happened.
+    if (CurrentParamStatus.isErrorStatus()) {
+      CurrentParamStatus = ParameterStatus::Escaped;
+    }
+  }
+
   void findAndReportNotCalledBranches(const CFGBlock *Parent, unsigned Index,
                                       bool IsEscape = false) {
     for (const CFGBlock *Succ : Parent->succs()) {
       if (!Succ)
         continue;
 
       if (getState(Succ).getKindFor(Index) == ParameterStatus::NotCalled) {
         assert(Parent->succ_size() >= 2 &&
void checkEscapee(const Expr *E) {
     if (const ParmVarDecl *Parameter = findReferencedParmVarDecl(E)) {
       checkEscapee(*Parameter);
     }
   }
 
   /// Check given parameter that was discovered to escape.
   void checkEscapee(const ParmVarDecl &Parameter) {
     if (auto Index = getIndex(Parameter)) {
-      ParameterStatus &CurrentParamStatus = CurrentState.getStatusFor(*Index);
-
-      if (CurrentParamStatus.getKind() == ParameterStatus::NotCalled) {
-        CurrentParamStatus = ParameterStatus::Escaped;
-      }
+      processEscapeFor(*Index);
     }
   }
 
   /// Mark all parameters in the current state as 'no-return'.
   void markNoReturn() {
     for (ParameterStatus &PS : CurrentState) {
       PS = ParameterStatus::NoReturn;
     }

clang/test/SemaObjC/warn-called-once.m

if (cond1) {
     // no-warning
   }
 
   if (cond2) {
     handler();
   }
 }
 
+- (void)test_escape_before_branch:(int)cond
+                   withCompletion:(void (^)(void))handler {
+  if (cond) {
+    filler();
+  }
+
+  void (^copiedHandler)(void) = ^{
+    handler();
+  };
+
+  if (cond) {
+    // no-warning
+    handler();
+  } else {
+    copiedHandler();
+  }
+}
+
+- (void)test_escape_after_branch:(int)cond
+                  withCompletion:(void (^)(void))handler {
+  if (cond) {
+    // no-warning
+    handler();
+  }
+
+  escape(handler);
+}
+
 @end