NagyDonat wrote: > By making `getState()` private/protected you effectively prove it at compile > time that the code no longer makes the mistake you're describing. I see this > as a much better thing to do than making a runtime test. Like, why would we > even be here if we didn't believe in the superiority of compile-time bug > prevention? đŸ˜… > > Also we're not even trying to prevent the `CallEvent`'s inner state from > going out of sync.
No, this commit _is_ trying to prevent the `CallEvent`'s inner state from going out of sinc. I am proposing a hybrid approach: 1. This commit ensures that the inner state of the `CallEvent` is up-to-date (for the EvalCall and PointerEscape callbacks that are targeted by this commit). 2. The planned followup commit will make `getState()` protected to limit access to the inner state of `CallEvent` and ensure that the analyzer works correctly even if the inner state of the `CallEvent` is obsolete. These two commits are mostly redundant with each other (either of them would fix 90% of the problems alone), but I see minor reasons for applying both of them: - If we make `getState()` protected but don't apply this commit the method `CXXInstanceCall::getDeclForDynamicType()` and the analogous tricky Objective-C logic – which I don't understand – could theoretically leak the obsolete parts of the state attached to the `CallEvent`. (I don't know whether it actually leaks problematic parts of the state, but I cannot rule it out.) - Also, I would prefer applying this commit from an aesthetic "Why not be accurate?" point of view – if we know the accurate state and can easily attach it to the `CallEvent`, then I prefer just attaching it instead of reasoning like "actually, the old state is inaccurate but not too inaccurate, so we can keep and use it..." - If we apply this commit but don't make `getState()` protected, then state attached to the `CallEvent` will be up-to-date at the start of the the EvalCall and PointerEscape callbacks (btw it is also up-to-date at the start of Pre/PostCall even without this commit), but for the sake of elegance and consistency I still don't want to see checkers that get the state from the `CallEvent` instead of getting the same state from the `CheckerContext`. https://github.com/llvm/llvm-project/pull/160707 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
