courbet added a comment.

In D153132#4651204 <https://reviews.llvm.org/D153132#4651204>, @courbet wrote:

> In D153132#4647557 <https://reviews.llvm.org/D153132#4647557>, @aaronpuchert 
> wrote:
>
>> In D153132#4431627 <https://reviews.llvm.org/D153132#4431627>, @courbet 
>> wrote:
>>
>>>> Is this actually required for the subsequent change? I don't see the 
>>>> connection.
>>>
>>> In the followup change, we have to check the returns after the enter and 
>>> exit CFG block are computed. We can't analyze the returns as they are seen 
>>> because , because what matters for the returns is the locks that are live 
>>> at the end of the function, not those that are live at the point where the 
>>> `return` happens.
>>
>> Is this still the case? Or do we not need this anymore.
>
> So the change we still actually need is for `checkAccess()` to take the fact 
> set as a parameter (because we're checking two different fact sets depending 
> on whether we're processing a return statement or not). I think that 
> design-wise it's better if `checkAccess` is in the `Analyzer` rather than the 
> `BuildLockset`, because `checkAccess()` no longer needs access to any state 
> within `BuildLockset`.

The patch without those base changes for reference: 
https://github.com/llvm/llvm-project/commit/11d3339daf6f3543d73292b307057769711bce2e.

>>> From a design perspective I think it might actually make more sens for them 
>>> to be in the analyzer as `warnIfMutexNotHeld` and friends actually inspects 
>>> quite a lot of the `Analyzer` state.
>>
>> On that I agree.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153132/new/

https://reviews.llvm.org/D153132

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to