aaron.ballman added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:25 + llvm::raw_svector_ostream Output(Buffer); + Output << "warning: Option not found '" << OptionName << "'\n"; + return std::string(Buffer); ---------------- aaron.ballman wrote: > I think the diagnostic text should probably not start with a capital letter. > Also, the name of the classes are confusing in that they say error but the > diagnostic is a warning. When I hear "error", the class name makes me think > this would stop compilation and give a nonzero result code from the program. Sorry, I was unclear with what I was looking for. It looks to me like the behavior of `OptionError` is to act more like a warning than an error in that a message is printed and execution of the tool continues, correct? If so, then I'd prefer to rename it to `OptionWarning` (unless you want to generalize it to be either a warning or an error with a return code, then `OptionDiagnostic` would be good), and all the subclasses to be `FooWarning`. Or did I misunderstand the design? Also, I like having the `warning:` in the message (though we could bikeshed if we want to make it look less like a check warning by doing something like `warning [clang-tidy command line]:`), but maybe that's being added by `OptionsView::logErrToStdErr()`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77085/new/ https://reviews.llvm.org/D77085 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
