Daniel599 updated this revision to Diff 267455.
Daniel599 added a comment.
Herald added subscribers: llvm-commits, dexonsmith.
Herald added a project: LLVM.
I have made the needed changes in order to detect a conflict in duplicated
fix-it of aliased checkers and also added a test for it.
Now I`ll try to work on combining aliased errors together, any tips regarding
this feature?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80753/new/
https://reviews.llvm.org/D80753
Files:
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
llvm/include/llvm/ADT/StringMap.h
Index: llvm/include/llvm/ADT/StringMap.h
===================================================================
--- llvm/include/llvm/ADT/StringMap.h
+++ llvm/include/llvm/ADT/StringMap.h
@@ -248,6 +248,24 @@
return count(MapEntry.getKey());
}
+ /// equal - check whether both of the containers are equal
+ bool operator==(const StringMap &RHS) const {
+ if (size() != RHS.size())
+ return false;
+
+ for (const auto &KeyValue : *this) {
+ auto FindInRHS = RHS.find(KeyValue.getKey());
+
+ if (FindInRHS == RHS.end())
+ return false;
+
+ if (!(KeyValue.getValue() == FindInRHS->getValue()))
+ return false;
+ }
+
+ return true;
+ }
+
/// insert - Insert the specified key/value pair into the map. If the key
/// already exists in the map, return false and ignore the request, otherwise
/// insert it and return true.
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t
+
+namespace std {
+template <typename>
+class initializer_list {
+public:
+ initializer_list() noexcept {}
+};
+
+template <typename T>
+class vector {
+public:
+ vector() = default;
+ vector(initializer_list<T>) {}
+
+ void push_back(const T &) {}
+ void push_back(T &&) {}
+
+ template <typename... Args>
+ void emplace_back(Args &&... args){};
+ ~vector();
+};
+} // namespace std
+
+class Foo {
+public:
+ Foo() : _num1(0)
+ // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init]
+ // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [hicpp-member-init]
+ // CHECK-MESSAGES: note: this fix will not be applied because an alias checker has already provided it, see 'cppcoreguidelines-pro-type-member-init'
+ {
+ _num1 = 10;
+ }
+
+ int use_the_members() const {
+ return _num1 + _num2;
+ }
+
+private:
+ int _num1;
+ int _num2;
+ // CHECK-FIXES: _num2{};
+};
+
+int should_use_emplace(std::vector<Foo> &v) {
+ v.push_back(Foo());
+ // CHECK-FIXES: v.emplace_back();
+ // CHECK-MESSAGES: warning: use emplace_back instead of push_back [hicpp-use-emplace]
+ // CHECK-MESSAGES: warning: use emplace_back instead of push_back [modernize-use-emplace]
+ // CHECK-MESSAGES: note: this fix will not be applied because an alias checker has already provided it, see 'hicpp-use-emplace'
+}
+
Index: clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t -- \
+//// RUN: -config='{CheckOptions: [ \
+//// RUN: {key: cppcoreguidelines-pro-type-member-init.UseAssignment, value: 1}, \
+//// RUN: ]}'
+
+class Foo {
+public:
+ Foo() : _num1(0)
+ // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [cppcoreguidelines-pro-type-member-init]
+ // CHECK-MESSAGES: note: 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: 'cppcoreguidelines-pro-type-member-init', 'hicpp-member-init'
+ // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [hicpp-member-init]
+ // CHECK-MESSAGES: note: 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: 'cppcoreguidelines-pro-type-member-init', 'hicpp-member-init'
+ {
+ _num1 = 10;
+ }
+
+ int use_the_members() const {
+ return _num1 + _num2;
+ }
+
+private:
+ int _num1;
+ int _num2;
+ // CHECK-FIXES: _num2;
+};
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -246,6 +246,7 @@
private:
void finalizeLastError();
void removeIncompatibleErrors();
+ void removeDuplicatedFixesOfAliasCheckers();
/// Returns the \c HeaderFilter constructed for the options set in the
/// context.
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -634,6 +634,8 @@
std::tuple<unsigned, EventType, int, int, unsigned> Priority;
};
+ removeDuplicatedFixesOfAliasCheckers();
+
// Compute error sizes.
std::vector<int> Sizes;
std::vector<
@@ -728,3 +730,63 @@
removeIncompatibleErrors();
return std::move(Errors);
}
+
+namespace {
+struct LessClangTidyErrorWithoutDiagnosticName {
+ bool operator()(const ClangTidyError *LHS, const ClangTidyError *RHS) const {
+ const tooling::DiagnosticMessage &M1 = LHS->Message;
+ const tooling::DiagnosticMessage &M2 = RHS->Message;
+
+ return std::tie(M1.FilePath, M1.FileOffset, M1.Message) <
+ std::tie(M2.FilePath, M2.FileOffset, M2.Message);
+ }
+};
+} // end anonymous namespace
+
+void ClangTidyDiagnosticConsumer::removeDuplicatedFixesOfAliasCheckers() {
+ using UniqueErrorSet =
+ std::set<ClangTidyError *, LessClangTidyErrorWithoutDiagnosticName>;
+ UniqueErrorSet UniqueErrors;
+
+ for (auto &Error : Errors) {
+ std::pair<UniqueErrorSet::iterator, bool> Inserted =
+ UniqueErrors.insert(&Error);
+
+ // If we already have an error like this, just with the different
+ // DiagnosticName
+ if (!Inserted.second) {
+ const llvm::StringMap<tooling::Replacements> &CandidateFix =
+ Error.Message.Fix;
+ const llvm::StringMap<tooling::Replacements> &CurrentFix =
+ (*Inserted.first)->Message.Fix;
+
+ if (!CandidateFix.empty() && !CurrentFix.empty()) {
+
+ // In case both of the checkers provide the exact same fix
+ // Remove its Fix since we don't need same fix twice
+ if (CandidateFix == CurrentFix) {
+ Error.Message.Fix.clear();
+ std::string FixAlreadyExists =
+ "this fix will not be applied because an alias checker has "
+ "already provided it, see '" +
+ (*Inserted.first)->DiagnosticName + "'";
+ Error.Notes.emplace_back(std::move(FixAlreadyExists));
+ } else {
+ 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. "
+ "Aliased checkers: '" +
+ (*Inserted.first)->DiagnosticName + "', '" +
+ Error.DiagnosticName + "'";
+
+ Error.Message.Fix.clear();
+ Error.Notes.emplace_back(AliasedCheckerFixConflict);
+ (*Inserted.first)->Message.Fix.clear();
+ (*Inserted.first)
+ ->Notes.emplace_back(std::move(AliasedCheckerFixConflict));
+ }
+ }
+ }
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
