aaron.ballman added a comment. In D71846#1800480 <https://reviews.llvm.org/D71846#1800480>, @njames93 wrote:
> In D71846#1800411 <https://reviews.llvm.org/D71846#1800411>, @aaron.ballman > wrote: > > > In D71846#1800401 <https://reviews.llvm.org/D71846#1800401>, @njames93 > > wrote: > > > > > In D71846#1800381 <https://reviews.llvm.org/D71846#1800381>, > > > @aaron.ballman wrote: > > > > > > > In D71846#1800344 <https://reviews.llvm.org/D71846#1800344>, @njames93 > > > > wrote: > > > > > > > > > I'm in two minds about issuing a warning when scope restrictions > > > > > prevent a fix. Do you think creating an option to enable or disable > > > > > emitting warnings for cases where the scope prevents a fix would be a > > > > > good idea? > > > > > > > > > > > > It's not uncommon for fixits to only be generated under specific > > > > circumstances, so I'm wondering what your concern is with warning when > > > > we can't provide a fixit? The cases I am thinking about all seem > > > > reasonable to diagnose (are true positives) without fixing, but maybe > > > > you have different circumstances in mind. > > > > > > > > > Right now an issue is raised for every else after return flag, but not > > > all else after return flags can be fixed due to declaration statements > > > and scope issues. My suggestion is that you can choose to warn about > > > those cases or not. For example a developer may want else after return > > > for when they need to limit the scope and getting a warning for it may be > > > undesirable. > > > > > > Okay, I can see the value in having an option for that -- especially given > > that silencing the diagnostic would add `// NOLINT` noise to the code. Do > > you think the option should default to diagnosing all cases, even ones > > without a fixit? > > > Definitely default to diagnosing everything, that's how my current > implementation looks right now. SGTM > Also is there a way to do options as bool because I don't particularly like > putting 1 for true in the config file Not to my knowledge. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71846/new/ https://reviews.llvm.org/D71846 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
