[PATCH] D84924: [clang-tidy] Added command line option `fix-notes`

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 21 04:16:05 PST 2020


njames93 added a comment.

In D84924#2446075 <https://reviews.llvm.org/D84924#2446075>, @aaron.ballman wrote:

> In D84924#2184132 <https://reviews.llvm.org/D84924#2184132>, @njames93 wrote:
>
>> This is very much a work in progress
>> Another direction I was thinking was only apply the fixes found in notes if there is exactly one fix attached to the notes in a diagnostic, instead of just applying the first one we find
>
> I was wondering the same thing here myself. If there's exactly one fix, then it's unambiguous as to what behavior you get. One (minor) concern I have about the current approach is with nondeterminism in diagnostic ordering where different runs may pick different fixes for the same code. I don't *think* we have any instances of this in Clang or clang-tidy, but users can add their own plugins (for instance to the clang static analyzer) that may introduce some small risk there. Do you have a reason why you picked one approach over the other?

Part of the reason for this approach is from this bug report https://bugs.llvm.org/show_bug.cgi?id=47971, where its pointed out that clang diags gets fix-its added in notes if the fixes will change behaviour or they aren't sure its going to actually fix the issue.
As clang-tidy also applies fixes reported from clang, it is wise to adhere to a similar level caution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84924



More information about the cfe-commits mailing list