njames93 added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp:20-21
+void ComparisonOperatorCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(functionDecl(hasAnyOverloadedOperatorName("==", "!=", "<",
+ "<=", ">",
">="))
+ .bind("operator"),
----------------
Is there a precedent to match spaceship operator?
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp:48-52
+ if (isa<CXXMethodDecl>(Operator)) {
+ diag(Operator->getLocation(), "%0 should not be member function")
+ << Operator;
+ return;
+ }
----------------
We should likely only warn on the canonical declaration
```lang=c++
struct A{
bool operator==(const A&) const; // Warn here
};
bool A::operator==(const A&) const { // Don't warn here
return true;
}```
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp:54-55
+
+ if (canThrow(Operator))
+ diagCanThrow(Operator);
+
----------------
This code should not be executed when running in pre c++11 mode, as noexcept
was added in c++11.
================
Comment at:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-comparison-operator.cpp:1
+// RUN: %check_clang_tidy %s cppcoreguidelines-comparison-operator %t
+
----------------
Once you rebase to trunk this test will fail without `--fix-notes` specified.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98692/new/
https://reviews.llvm.org/D98692
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
