hyeongyukim added a comment. In D105169#3319773 <https://reviews.llvm.org/D105169#3319773>, @dblaikie wrote:
> 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) Sure, I'll add some tests. By the way, is it right to change the flag's name to `-[no-]enable-noundef-analysis`? or would it be better to use `-[no-]noundef-analysis` as @MaskRay suggested? I prefer to use `-[no-]enable-noundef-analysis` to maintain backward compatibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105169/new/ https://reviews.llvm.org/D105169 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
