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 Priority; }; + removeDuplicatedFixesOfAliasCheckers(); + // Compute error sizes. std::vector 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; + UniqueErrorSet UniqueErrors; + + for (auto &Error : Errors) { + std::pair Inserted = + UniqueErrors.insert(&Error); + + // If we already have an error like this, just with the different + // DiagnosticName + if (!Inserted.second) { + const llvm::StringMap &CandidateFix = + Error.Message.Fix; + const llvm::StringMap &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)); + } + } + } + } +} 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/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 +class initializer_list { +public: + initializer_list() noexcept {} +}; + +template +class vector { +public: + vector() = default; + vector(initializer_list) {} + + void push_back(const T &) {} + void push_back(T &&) {} + + template + 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 &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: 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.