ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp:46
+
+ StringRef Message = "no explanation";
+ if (Case.Explanation) {
----------------
ymandel wrote:
> 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.
Ah, so all the options are bad. I can see why you had this design in
transformers in the first place.
I wonder if we should check the explanations are always set for rewrite rules
inside the clang-tidy transformation?
================
Comment at: clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h:1
+//===---------- TransformerClangTidyCheck.h - clang-tidy
------------------===//
+//
----------------
ymandel wrote:
> 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?
Let's stick with Dmitri's suggestion. He has much more experience with LLVM
than I do :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61386/new/
https://reviews.llvm.org/D61386
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
