[clang] [analyzer] Don't invalidate pointee of pointer-to-const on callback arguments (PR #225489)
Donát Nagy via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 21 11:47:41 PDT 2026
NagyDonat wrote:
> > the one found by claude;
>
> This would be a small change in one commit.
My bad, I should have been more specific :sweat_smile: In fact Claude has found _two_ bugs:
- The small change that you speak about, the off-by-one error in `if (FName == "funopen") { if (Call.getNumArgs() >= 4 && Call.getArgSVal(4).isConstant(0))` – you are completely right that this can and should be postponed to a separate commit (because this is a preexisting issue that is independent of the rest)
- The more complex issue that I'm speaking about, where this PR introduces a memory leak false positive on the Objective-C++ code example shown in [this comment](https://github.com/llvm/llvm-project/pull/225489#issuecomment-6314962891) – this is a regression that would be introduced by this PR, so we shouldn't postpone it to a follow-up PR.
> This is a more complex change but if I do dedicated commits for it and nicely explain them in a comment that might help with following each step.
This is exactly what I would've proposed a few years ago, but recently I started to see the value of a fresh start.
Abandoning these commits may appear as if we're wasting something valuable. However, the real value gained in this review process is that we have a better understanding of the possibilities, challenges and pitfalls – and the shaky-questionable commits are more of a liability than an asset.
This first PR just explores the area, as if the scouts of an army were exploring a swamp that they need to cross. Instead of "keeping" the scouts' meandering path and preparing it for the passage the whole army, it is often much better to use the information gained in the exploration to select and finalize a different, easier route.
> > maybe it is better if we switch roles: I create the PRs and you verify that I'm not overlooking some corner case.
>
> If you disagree with my approach in this comment then we can go with this suggestion. I want to make sure that 1) we can follow through the reasoning on both sides 2) do not take more review time than needed from you and steakhal. And of course having a change that is correct.
I would like to switch into a coding role, because – due to the complexity of the situation – I cannot "think ahead" and propose a large-scale plan where you can effectively fill in the details. If I'm coding, I can try uncertain ideas, iterate quickly and hopefully translate my vague understanding of the area more effectively.
I think implementing this myself (with reviews to catch my mistakes!) would take less time from me than completing a review. (And if I'm overestimating my abilities, then this will be a useful learning experience :sweat_smile:)
https://github.com/llvm/llvm-project/pull/225489
More information about the cfe-commits
mailing list