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:
> 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?
> Ah, so all the options are bad. I can see why you had this design in 
> transformers in the first place.
Heh. indeed.

> I wonder if we should check the explanations are always set for rewrite rules 
> inside the clang-tidy transformation?> Quoted Text

I would have thought so, but AFAIK, most folks who write one-off 
transformations use clang-tidy, rather than writing a standalone tool. They 
just ignore the diagnostics, i gather.  Transformer may shift that somewhat if 
we improve the experience of writing a (throwaway) standalone tool, but for the 
time being I think we can't assume that.



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

Reply via email to