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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

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)



================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp:47
+  const auto *Operator = Result.Nodes.getNodeAs<FunctionDecl>("operator");
+  const FunctionDecl *CanonOperator = Operator->getCanonicalDecl();
+
----------------
It might just be better to return here if the matched result isn't the same as the canonical declaration. 


================
Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ComparisonOperatorCheck.cpp:54-55
+
+  if (canThrow(Operator))
+    diagCanThrow(Operator);
+
----------------
njames93 wrote:
> This code should not be executed when running in pre c++11 mode, as noexcept was added in c++11.
I was saying the other checks can run in c++98 mode but then wrap this in an if block for c++11


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98692