[PATCH] D82317: [Clang/Test]: Update tests where `noundef` attribute is necessary

Wed Aug 12 11:43:29 PDT 2020

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