Daniel599 updated this revision to Diff 267478.
Daniel599 added a comment.
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.
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
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80753/new/
https://reviews.llvm.org/D80753
Files:
clang-tools-extra/clang-tidy/ClangTidy.cpp
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
clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.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-reports.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/duplicate-reports.cpp
+++ 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;
}
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,48 @@
+// 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,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<Foo> &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]
+}
+
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,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 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
@@ -48,6 +48,7 @@
bool IsWarningAsError);
bool IsWarningAsError;
+ std::vector<std::string> EnabledDiagnosticAliases;
};
/// Contains displayed and ignored diagnostic counters for a ClangTidy
@@ -246,6 +247,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,61 @@
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;
+
+ auto IT = Errors.begin();
+ while (IT != Errors.end()) {
+ ClangTidyError &Error = *IT;
+ std::pair<UniqueErrorSet::iterator, bool> Inserted =
+ UniqueErrors.insert(&Error);
+
+ // Unique error, we keep it and move along
+ if (Inserted.second) {
+ ++IT;
+ } else {
+ ClangTidyError &ExistingError = **Inserted.first;
+ const llvm::StringMap<tooling::Replacements> &CandidateFix =
+ Error.Message.Fix;
+ const llvm::StringMap<tooling::Replacements> &ExistingFix =
+ (*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. "
+ "Aliased checkers: '" +
+ (*Inserted.first)->DiagnosticName + "', '" + Error.DiagnosticName +
+ "'";
+
+ // In case of a conflict, don't sugeest any fix-it
+ (*Inserted.first)->Message.Fix.clear();
+ (*Inserted.first)
+ ->Notes.emplace_back(std::move(AliasedCheckerFixConflict));
+ }
+
+ if (Error.IsWarningAsError)
+ ExistingError.IsWarningAsError = true;
+
+ // Since it is the same error, we should take it alias and remove it
+ ExistingError.EnabledDiagnosticAliases.emplace_back(Error.DiagnosticName);
+ IT = Errors.erase(IT);
+ }
+ }
+}
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -121,6 +121,8 @@
{
auto Level = static_cast<DiagnosticsEngine::Level>(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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
