[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.
Yitzhak Mandelbaum via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed May 22 05:32:52 PDT 2019
ymandel marked 4 inline comments as done.
ymandel added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:46
+
+ StringRef Message = "no explanation";
+ if (Case.Explanation) {
----------------
ilya-biryukov wrote:
> The users will see this for every case without explanation, right?
> I'd expect the rules without explanation to be somewhat common, maybe don't show any message at all in that case?
There's no option to call `diag()` without a message. We could pass an empty string , but that may be confusing given the way the message is concatenated here:
https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp#L204
So, no matter what, there will be some message to go w/ the diagnostic. I figure that being explicit about the lack of explanation is better than an empty string, but don't feel strongly about this.
================
Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h:1
+//===---------- TransformerClangTidyCheck.h - clang-tidy ------------------===//
+//
----------------
ilya-biryukov wrote:
> NIT: maybe use `TransformerCheck` for brevity? I'm not a `clang-tidy` maintainer, though, so not sure whether that aligns with the rest of the code.
It was TransformerTidy and Dmitri suggested I be explicit. I think TransformerCheck is better than TransformerTidy, but I also see the argument in spelling it out. WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61386/new/
https://reviews.llvm.org/D61386
More information about the cfe-commits
mailing list