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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits