PiotrZSL added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:29
+
+bool isPotentialSymmetricBinaryOperator(OverloadedOperatorKind Kind) {
+ return llvm::is_contained(
----------------
again, I would probably try to generate simple switch with those macros., and
this function should be static.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:48
+
+bool isComparisonOperator(OverloadedOperatorKind Kind) {
+ std::bitset<NUM_OVERLOADED_OPERATORS> BitSetCompOps{};
----------------
For me this function is overkill, I would use simple switch and let compiler
optimize it. except that should be static.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:62-68
+AST_MATCHER(CXXMethodDecl, isPotentialSymmetricBinaryOperator) {
+ return isPotentialSymmetricBinaryOperator(Node.getOverloadedOperator());
+}
+
+AST_MATCHER(CXXMethodDecl, isComparisonOperator) {
+ return isComparisonOperator(Node.getOverloadedOperator());
+}
----------------
put matchers into anonymous namespace....
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:164
+ StringRef Name, ClangTidyContext *Context)
+ : TransformerClangTidyCheck(
+ makeRewriteRule(Context->getLangOpts().CPlusPlus20), Name, Context)
{}
----------------
Consider writing this check as an normal simple one without using
TransformerClangTidyCheck as base. After that half of this code won't be
needed, and it will be very easy to maintain/fix/extend it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128861/new/
https://reviews.llvm.org/D128861
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
