[PATCH] D98692: [clang-tidy] Add cppcoreguidelines-comparison-operator

Wed Mar 17 05:08:34 PDT 2021

nullptr.cpp added a comment.

In D98692#2631182 <https://reviews.llvm.org/D98692#2631182>, @njames93 wrote:

> While we're here, I'm not sure if there is a cppcoreguideline for it, but parameters to comparison functions should be passed by const ref usually, or by value if they are cheap to copy. Maybe there's precedent to extend this to flag parameters that take non const reference, R value reference or by value if not cheap to copy(There's bound to be a function somewhere in clang tidy that determines if a type is a cheap copy)

The relevant rule is F.16: For "in" parameters, pass cheaply-copied types by value and others by reference to const <https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#f16-for-in-parameters-pass-cheaply-copied-types-by-value-and-others-by-reference-to-const>.
This should be done in another patch dedicated to parameter passing.

================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp:47
+  const auto *Operator = Result.Nodes.getNodeAs<FunctionDecl>("operator");
+  const FunctionDecl *CanonOperator = Operator->getCanonicalDecl();
+
----------------
njames93 wrote:
> It might just be better to return here if the matched result isn't the same as the canonical declaration. 
Why only warn on the canonical declaration?
Clients need to find the definition and other declarations by themselves. I think it's better to warn about everything found here.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98692/new/

https://reviews.llvm.org/D98692