https://github.com/steakhal requested changes to this pull request.

According to the PR title, this should be NFC.
That implies that behavior (features) are neither dropped, nor added.
I should go even stronger, the intentions of calling something NFC is that it's 
transparent to existing bugs.

It's hard to spot, but thanks to AI, it raised my attention to that the new 
file has this `VisitCXXDefaultArgExpr` while it didn't at the previous place.

There are other changes; sometimes more subtle than this, that are hard to spot.
Could you split this? I'm sure this was incidental but one should be very 
careful of claiming that a patch is NFC because if it's merged and breaks build 
bots for semantic changes then people could claim that we are not following 
LLVM practices.

As a reviewer, I want to confidently rely on NFC claims because that is the 
only way one could scale up my attention to large changes and turn their review 
complexity to `O(1)`, and on the flip side, focus the spared review capacity on 
the functional changes, what truly matters.

https://github.com/llvm/llvm-project/pull/188648
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to