diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -122,6 +122,8 @@ { auto Level = static_cast(Error.DiagLevel); std::string Name = Error.DiagnosticName; + if (!Error.EnabledDiagnosticAliases.empty()) + Name += "," + llvm::join(Error.EnabledDiagnosticAliases, ","); if (Error.IsWarningAsError) { Name += ",-warnings-as-errors"; Level = DiagnosticsEngine::Error; diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -48,6 +48,7 @@ bool IsWarningAsError); bool IsWarningAsError; + std::vector EnabledDiagnosticAliases; }; /// Contains displayed and ignored diagnostic counters for a ClangTidy @@ -246,6 +247,7 @@ private: void finalizeLastError(); void removeIncompatibleErrors(); + void removeDuplicatedDiagnosticsOfAliasCheckers(); /// Returns the \c HeaderFilter constructed for the options set in the /// context. diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -27,6 +27,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/Regex.h" +#include "llvm/Support/FormatVariadic.h" #include #include using namespace clang; @@ -634,6 +635,8 @@ std::tuple Priority; }; + removeDuplicatedDiagnosticsOfAliasCheckers(); + // Compute error sizes. std::vector Sizes; std::vector< @@ -728,3 +731,59 @@ 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::removeDuplicatedDiagnosticsOfAliasCheckers() { + using UniqueErrorSet = + std::set; + UniqueErrorSet UniqueErrors; + + auto IT = Errors.begin(); + while (IT != Errors.end()) { + ClangTidyError &Error = *IT; + std::pair Inserted = + UniqueErrors.insert(&Error); + + // Unique error, we keep it and move along. + if (Inserted.second) { + ++IT; + } else { + ClangTidyError &ExistingError = **Inserted.first; + const llvm::StringMap &CandidateFix = + Error.Message.Fix; + const llvm::StringMap &ExistingFix = + (*Inserted.first)->Message.Fix; + + if (CandidateFix != ExistingFix) { + + // In case of a conflict, don't suggest any fix-it. + ExistingError.Message.Fix.clear(); + 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 ('{0}', '{1}') or " + "ensure they are both configured the same", + ExistingError.DiagnosticName, Error.DiagnosticName) + .str()); + } + + if (Error.IsWarningAsError) + ExistingError.IsWarningAsError = true; + + // Since it is the same error, we should take it as alias and remove it. + ExistingError.EnabledDiagnosticAliases.emplace_back(Error.DiagnosticName); + IT = Errors.erase(IT); + } + } +} diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-conflicted-fixes-of-alias-checkers.cpp @@ -0,0 +1,23 @@ +// 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,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 ('cppcoreguidelines-pro-type-member-init', 'hicpp-member-init') or ensure they are both configured the same + { + _num1 = 10; + } + + int use_the_members() const { + return _num1 + _num2; + } + +private: + int _num1; + int _num2; + // CHECK-FIXES: _num2; +}; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-fixes-of-alias-checkers.cpp @@ -0,0 +1,39 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-member-init,hicpp-member-init,modernize-use-emplace,hicpp-use-emplace %t + +namespace std { + +template +class vector { +public: + void push_back(const T &) {} + void push_back(T &&) {} + + template + void emplace_back(Args &&... args){}; +}; +} // namespace std + +class Foo { +public: + Foo() : _num1(0) + // CHECK-MESSAGES: warning: constructor does not initialize these fields: _num2 [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{}; +}; + +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,modernize-use-emplace] +} + diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp --- a/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp @@ -2,8 +2,7 @@ void alwaysThrows() { int ex = 42; - // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp] - // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err61-cpp] + // CHECK-MESSAGES: warning: throw expression should throw anonymous temporary values instead [cert-err09-cpp,cert-err61-cpp] throw ex; } diff --git a/llvm/include/llvm/ADT/StringMap.h b/llvm/include/llvm/ADT/StringMap.h --- a/llvm/include/llvm/ADT/StringMap.h +++ b/llvm/include/llvm/ADT/StringMap.h @@ -248,6 +248,26 @@ 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; + } + + bool operator!=(const StringMap &RHS) const { return !(*this == RHS); } + /// 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. diff --git a/llvm/unittests/ADT/StringMapTest.cpp b/llvm/unittests/ADT/StringMapTest.cpp --- a/llvm/unittests/ADT/StringMapTest.cpp +++ b/llvm/unittests/ADT/StringMapTest.cpp @@ -387,6 +387,70 @@ ASSERT_EQ(B.count("x"), 0u); } +TEST_F(StringMapTest, EqualEmpty) { + StringMap A; + StringMap B; + ASSERT_TRUE(A == B); + ASSERT_FALSE(A != B); + ASSERT_TRUE(A == A); // self check +} + +TEST_F(StringMapTest, EqualWithValues) { + StringMap A; + A["A"] = 1; + A["B"] = 2; + A["C"] = 3; + A["D"] = 3; + + StringMap B; + B["A"] = 1; + B["B"] = 2; + B["C"] = 3; + B["D"] = 3; + + ASSERT_TRUE(A == B); + ASSERT_TRUE(B == A); + ASSERT_FALSE(A != B); + ASSERT_FALSE(B != A); + ASSERT_TRUE(A == A); // self check +} + +TEST_F(StringMapTest, NotEqualMissingKeys) { + StringMap A; + A["A"] = 1; + A["B"] = 2; + + StringMap B; + B["A"] = 1; + B["B"] = 2; + B["C"] = 3; + B["D"] = 3; + + ASSERT_FALSE(A == B); + ASSERT_FALSE(B == A); + ASSERT_TRUE(A != B); + ASSERT_TRUE(B != A); +} + +TEST_F(StringMapTest, NotEqualWithDifferentValues) { + StringMap A; + A["A"] = 1; + A["B"] = 2; + A["C"] = 100; + A["D"] = 3; + + StringMap B; + B["A"] = 1; + B["B"] = 2; + B["C"] = 3; + B["D"] = 3; + + ASSERT_FALSE(A == B); + ASSERT_FALSE(B == A); + ASSERT_TRUE(A != B); + ASSERT_TRUE(B != A); +} + struct Countable { int &InstanceCount; int Number;