[PATCH] D80753: [clang-tidy] remove duplicate fixes of alias checkers

Daniel via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 29 14:14:28 PDT 2020


Daniel599 added a comment.

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

> In D80753#2061565 <https://reviews.llvm.org/D80753#2061565>, @njames93 wrote:
>
> > It may be worth verifying that the fix-its are identical too, multiple versions of a check could be running with differing options resulting in different fix-its generated, in that case we should let clang-tidy disable any conflicting fixes for us.
> >  Side note would it not be nicer to just group the diagnostics into one, thinking either of these ways
> >
> >   CHECK-MESSAGES: warning: use emplace_back instead of push_back [hicpp-use-emplace, modernize-use-emplace]
> >   CHECK-MESSAGES: warning: use emplace_back instead of push_back [hicpp-use-emplace] [modernize-use-emplace]
> >
> > This would result in cleaner diagnostics emitted and remove the need for that note.
>
>
> I think this is a nice approach. It also helps the case where a user sees a false positive diagnostic and tries to disable it in their config file. Under the removing duplicates behavior, the user would have a harder time discovering what tests to disable, but under the above approach, they'd have all the information they need up front.


**Regarding different fix-its generated: **
you are right, in order to fix it, I had to add `StringMap::operator ==` (soon I`ll update the diff), I hope it's OK.
I added the option cppcoreguidelines-pro-type-member-init.UseAssignment to check it, but the result I got was something like:

  int _num2 = 0{};

I think that if the fix-its aren't the same, both of them should be disabled with warning about it, what do you think?

**Regarding placing all errors of all aliases in one error:**
My original goal was to fix the bug that was introduce since clang-tidy-9, what you are suggesting sounds really cool, but I am still new to this code,
Any idea how can I implement such feature? also what should I do if the options of the alias checks aren't the same?


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

https://reviews.llvm.org/D80753





More information about the cfe-commits mailing list