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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to