[PATCH] D105169: [Clang/Test]: Rename enable_noundef_analysis to disable-noundef-analysis and turn it off by default

Mon Feb 14 10:06:29 PST 2022

dblaikie added a comment.

In D105169#3315009 <https://reviews.llvm.org/D105169#3315009>, @MaskRay wrote:

> It may not be worth changing now, but I want to mention: it's more conventional to have a `BoolOption` which adds `-[no-]noundef-analysis`. Since both positive and negative forms exist. When we make the default switch, existing users don't need to change the option. After the option becomes quite stable and the workaround is deemed not useful, we can remove the CC1 option.

+1 to this (changing the name and the default at the same time makes migrations a bit more difficult - if the default is changed without renaming (by having both positive and negative flag names) then users can adopt their current default explicitly with no change ahead of picking up the patch that changes the default) & also this flag seems to have no tests? Could you (@hyeongyukim ) add some frontend test coverage for the flag - and yeah, maybe consider giving it a name that has both explicit on/off names, as @maskray suggested? (I think that's useful even after the default switch - since a user might want to override a previous argument on the command line, etc)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105169