njames93 added a comment. 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. Also is there a way to do options as bool because I don't particularly like putting 1 for true in the config file 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
