balazs-benics-sonarsource wrote:
> As I thought a bit more about the reorganization that I suggested, I realized
> that it can be summarized as **we should synchronize adding the default
> `Unknown` binding and calling `escapeValue`** -- because they correspond to
> the two end-points of the same "_this_ value is stored at _this_ memory
> region" connection which wasn't properly recorded.
>
> Of course there is some asymmetry in that `escapeValue` must escape each
> value individually (or perhaps in a loop with `escapeValues`), but the
> default binding to `Unknown` region is (if I understand correctly) a proper
> stand-in for _all_ the connections from that side. This could be simply
> handled as "`if (`there is no default `Unknown` binding`) {` create one `}`"
> -- but if this happens to cause performance issues, then a boolean
> `didCreateDefaultUnknownBinding` can be used to cache the result.
AFAIK it's not possible (or rather would be ugly) to tie "adding the default
unknown binding" to `escapeValues`, because there may be multiple
`escapeValues` call in the recursive callstack, while popping the frames until
we leave the virtual `.*Bind.*` API.
For example, while binding a compound val like this: `{{{a,b}, {c,d}}, {e},
{f,g}}`, we may give up at `c`, which means, while leaving the method handling
the bind of `{c,d}`, will react to `hasExhaustedBindingLimit()`, and escape
only `d`. Then the parent frame of the recursive descent would conclude that it
finished binding `{c,d}`, and return. Same goes for its parent: `{{a,b},
{c,d}}`, but then it checks `hasExhaustedBindingLimit()` and escapes `{e}` and
`{f,g}`.
If we didn't add the default binding Unknown earlier, at this point it would be
difficult to recover the memory region to the specific element from which the
escapes happened, aka. the memregion of the access pattern to `c` - unless we
of course store that MemRegion in some side storage, and employ some RAII
technique to ensure that it's bound in the end.
I don't think it is worth this complexity.
I think I'm finished, I looked at everything again, and I still believe it's as
good as it gets.
I'm happy to be challenged, but I don't think I'd spend too much time on this.
Feel free to directly push to this branch to demonstrate what you would propose.
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
