guiand added a comment.

In D82317#2212145 <https://reviews.llvm.org/D82317#2212145>, @jdoerfert wrote:

> TBH, I don't see how this solves any problem. It just makes it a problem for 
> someone in the future... (FWIW, I say this being in full support of `noundef`)

That's true. At the same time, it might allow us to have a more baby-steps 
approach: be able to use the attribute now, update tests in their own time, and 
hopefully amortize the effect on downstream forks as @jrtc27 mentioned.

But the work for updating the tests is done now, in this patch, and as time 
goes on more and more of this work will need to be duplicated as the tests 
change every day.

There's one other option here, which has been mentioned a few times but I'll 
flesh it out for the purposes of discussion.

- Instead of updating the default test configuration to use 
`-disable-noundef-analysis`, we update every failing test (due to noundef) to 
use that flag in the RUN lines. This unblocks the clang noundef change.
- Then I can split up this patch into many smaller patches, probably organized 
by subfolder of `clang/test`, and these smaller patches would remove the 
`-disable-noundef-analysis` flags and add in the relevant `noundef` attributes.
- Test authors which don't want theirs changed could voice that they want to 
just keep the masking flag for their files.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82317/new/

https://reviews.llvm.org/D82317

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to