================
@@ -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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
