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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
