https://github.com/aaronpuchert approved this pull request.
It could be argued that the issue here is something different, namely that we don't add the scoped lock in the return slot to the expected function exit set. But this is likely not enough: * We don't compare the set of underlying mutexes of a scope at (real) join points, and there is no very compelling reason to do that at the moment: scopes are assigned their underlying mutexes at the time of construction, and we don't support changing them throughout the lifetime of the scope. So if both branches in a join have the same scope object, it comes from the same declaration and thus also initialization. So it will manage the same set of mutexes regardless of where we come from. * The status of a scoped capability roughly tracks its lifetime,¹ which makes it not so interesting by itself for the analysis. Because we only support local variables, the lifetime matches the extent of a lexical (block) scope. So we already know that some scope object is going to be returned, and that can only be the only scope object still alive at the end of the function. * At some point we should introduce a check that the set of managed capabilities of the returned scope matches the set of capabilities mentioned in attributes on the function. Since @malek203 is working on something similar I think we might see this relatively soon. However, this seems orthogonal to checking the status of managed mutexes, because as mentioned we don't generally want to check them on join points, just at the end of a function. Maybe we'll go back to comparing scoped capabilities, but let's see. For now this seems like the right change, and it should be easy to revert should we decide to move the differences of join point handling into the scoped capability handling instead. So this looks good to me, but let's wait if @AaronBallman has anything to add. ¹With exceptions: if they're constructed through a constructor without annotation we don't see the beginning of their lifetime, if the destructor has no annotation we don't see the end of it. But not having such annotations does not seem like a valid use case, and at some point we might change the behavior to strictly follow the lifetime of the object. https://github.com/llvm/llvm-project/pull/105526 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
