[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)

[Donát Nagy via cfe-commits]

================
@@ -2782,6 +2865,8 @@ RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B,
 
       if (VI == VE)
         break;
+      if (NewB.hasExhaustedBindingLimit())
+        return NewB.escapeValues(VI, VE);
----------------
NagyDonat wrote:

This is confusing to read because the `return *this;` pattern is very rare in the analyzer (I haven't seen it anywhere else) and I would strongly expect that a call like `NewB.escapeValues(VI, VE);` returns either `void` or (if I see that its return value is used, then) a boolean success/failure.

Please switch to an explicit syntax like
```cpp
if (NewB.hasExhaustedBindingLimit()) {
  NewB.escapeValues(VI, VE);
  return NewB;
}
```
or, if you really want to use `return *this;`, then at least rename the method to something like `withValuesEscaped()`. (That name would be still a bit misleading, because it would suggest that you're returning a mutated variant of an immutable object; but at least it would show the fact that the return value is the bindings object.)

-------

By the way, I think the `return *this;` pattern can be a very nice style, but only if it's applied consistently, because then the reader can expect that "ok, this method would naturally return `void`, so it will in fact return `*this`". (Also, its real strength is when it's needed for chained method calls -- just merging the method call with a return statement is not that important.)

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