aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM! ================ Comment at: clang-tools-extra/test/clang-tidy/infrastructure/verify-config.cpp:12 + +// CHECK-VERIFY-DAG: command-line option '-config': warning: Unknown Check 'readability-else-after-ret'; did you mean 'readability-else-after-return' +// CHECK-VERIFY-DAG: command-line option '-config': warning: Unknown Check Option 'modernize-lop-convert.UseCxx20ReverseRanges'; did you mean 'modernize-loop-convert.UseCxx20ReverseRanges' ---------------- njames93 wrote: > aaron.ballman wrote: > > njames93 wrote: > > > aaron.ballman wrote: > > > > It's unfortunate that `warning: ` appears in the middle of the > > > > diagnostic as opposed to at the start. I wonder if this can be reworked > > > > to say: `warning: Unknown check 'whatever'; did you mean 'whatever'? > > > > [-verify-config]` or something? > > > > > > > > Also, no test coverage for the error case and for the unknown check > > > > case where there's no closest match worth talking about. > > > Warning appearing in the middle is kind of by design as when clang emits > > > diagnostics, the source location is the first thing emitted, then the > > > type(warning|error). In this case `command-line option '-config'` is the > > > "source location" > > > > > > > Also, no test coverage for the error case. > > > I'm not 100% on that, there is no checking when the check glob is > > > actually built and I haven't figured out how to break it. > > > then the type(warning|error). In this case command-line option '-config' > > > is the "source location" > > > > Ah, thanks for explaining your logic. FWIW, Clang uses a different style > > for that (`<command line>`): https://godbolt.org/z/f1rdsbjqe Perhaps we > > should follow suit? (In fact, we could use the diagnostic engine for this > > instead of emitting to the error stream ourselves?) > The issue is, we don't have the actual source information right now to print > better error messages. After some off-list discussion (thanks @njames93!) I'm retracting my concerns here for the moment because there are larger issues with the diagnostics engine to be solved first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127446/new/ https://reviews.llvm.org/D127446 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
