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

Nathan James via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat May 30 15:54:13 PDT 2020


njames93 added a comment.

In D80753#2065067 <https://reviews.llvm.org/D80753#2065067>, @Daniel599 wrote:

> Thank you @njames93, I already started and took a bit different approach.
>  In case of a conflict, leaving it to clang-tidy itself didn't help as it added both of the fix-its together resulting in `= 0{};`, so I thought it will be better to disable both of them.


Fair enough, your solution looks a lot nicer than what I suggested anyway.

> Sadly I didn't find 3 aliased checkers which can be configured to produce different code so I can't check this edge case.
>  Please let me know if another changes are needed for this patch

You could create a unit test for 3 aliased checks with different options, however if there isn't a check that users can use to reproduce that edge case there is no point in worrying about it.



================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:765-778
+          (*Inserted.first)->Message.Fix;
+
+      if (!(CandidateFix == ExistingFix)) {
+        std::string AliasedCheckerFixConflict =
+            "cannot apply fix-it because an alias checker has "
+            "suggested a different fix-it, please remove one of the checkers "
+            "or make sure they are both configured the same. "
----------------
`s/(*Inserted.first)->/ExistingError.`


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:779
+        (*Inserted.first)
+            ->Notes.emplace_back(std::move(AliasedCheckerFixConflict));
+      }
----------------
```
        ExistingError.Notes.emplace_back(llvm::formatv(
            "cannot apply fix-it because an alias checker has "
            "suggested a different fix-it, please remove one of the checkers "
            "or make sure they are both configured the same. "
            "Aliased checkers: '{0}', '{1}'",
            ExistingError.DiagnosticName, Error.DiagnosticName).str());```


================
Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:51
   bool IsWarningAsError;
+  std::vector<std::string> EnabledDiagnosticAliases;
 };
----------------
Just ignore this, but do you ever get so bored and feel the need to save 24 bytes... https://gist.github.com/njames93/aac662ca52d345245e5e6f5dbc47d484 :)


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

https://reviews.llvm.org/D80753





More information about the llvm-commits mailing list