aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from a few small nits, but please wait a bit for @njames93 in case
they have final thoughts.
================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:758
+
+ // Unique error, we keep it and move along
+ if (Inserted.second) {
----------------
Add full stops to the end of your comments (here and elsewhere).
================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:777
+ "or make sure they are both configured the same. "
+ "Aliased checkers: '{0}', '{1}'",
+ ExistingError.DiagnosticName, Error.DiagnosticName)
----------------
Daniel599 wrote:
> njames93 wrote:
> > You don't really need the Aliased checkers note as that is wrapped in the
> > initial diagnostic message.
> > Also if there was a check that could spew out 3 different fix-its for the
> > same diagnostic, this would result in the duplication note being made
> > twice, maybe the notes should be cleared too?
> regarding your comment about 3 fix-it, as we walked before, there isn't a
> case like that (I didn't find any) as I wanted to make a test out of it.
> I added the last line as it would show who are the two that conflict
> (supporting the future case of more than 2 aliased checkers),
> I can remove it if it doesn't help, let me know.
>
> > maybe the notes should be cleared too?
> I am open for suggestions about how to re-write this message :)
> I also think it could be better
>
I would reword it to be a run-on sentence: `cannot apply fix-it because an
alias checker has suggested a different fix-it; please remove one of the
checkers ('{0}', '{1}') or ensure they are both configured the same`
This also nicely removes the nit about terminating punctuation in a diagnostic.
================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:250
void removeIncompatibleErrors();
+ void removeDuplicatedFixesOfAliasCheckers();
----------------
Daniel599 wrote:
> maybe I need to rename this method since now it's removing the errors also,
> I`ll do it when I get back from work.
I think this should still be renamed. How about
`removeDuplicatedDiagnosticsOfAliasCheckers()`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80753/new/
https://reviews.llvm.org/D80753
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
