mattbeardsley added a comment.

Thanks for your reply and sorry about my very sluggish reply!
I am looking into updating the docs as you suggested, and that got me looking 
at this doc page 
<https://clang.llvm.org/extra/clang-tidy/Contributing.html#writing-a-clang-tidy-check>.
 Interestingly, that doc page's version of the add_new_check.py template 
doesn't have the `Note` diagnostic. So that got me digging into the history.
It seems like 2 commits are at odds with each other about what the right thing 
to do is with `FixItHint`s w.r.t. `Note` vs `Warning` diagnostics.

1. This commit 
<https://github.com/llvm/llvm-project/commit/f2879d8a4877eafcdb12c852030746d175f8abbd>
 says that "fix descriptions and fixes are emitted via diagnostic::Note (rather 
than attaching the main warning diagnostic)."  - And note how that commit 
changes add_new_check.py - adding the `Note` diagnostic!
2. This commit 
<https://reviews.llvm.org/rGabbe9e227ed31e5dde9bb7567bb9f0dd047689c6> seems to 
suggest that Note diagnostics should //not// be used to provide the "standard" 
fix (and generally enforces that by `--fix-notes` being off by default)

It looks like nearly all existing check implementations disagree with the 
former, and agree with the latter.
Those two commits appear to be at odds with each other on the semantics of 
`Note` diagnostics - any opinions on how to reconcile that apparent 
disagreement?

My (limited) perspective is:
Since `--fix-notes` is off by default, it seems like `FixIt`s on `Note` 
diagnostics are pretty well enforced to be used for secondary/uncertain 
suggestions, while `FixIt`s on `Warning` diagnostics are for the primary 
fix(es). 
That seems to rule out accepting the guidance of first commit w.r.t. attaching 
fixes to `Note` diagnostics given the `--fix-notes` flag imposes that such 
note-fixes are implicitly secondary to warning-fixes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109210/new/

https://reviews.llvm.org/D109210

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to