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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
