njames93 added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:24-34
+using transformer::applyFirst;
+using transformer::cat;
+using transformer::changeTo;
+using transformer::insertBefore;
+using transformer::makeRule;
+using transformer::name;
+using transformer::node;
----------------
Just bring the whole namespace into scope.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:56
+bool isComparisonOperator(OverloadedOperatorKind Kind) {
+ return llvm::is_contained(
+ std::initializer_list<OverloadedOperatorKind>{
----------------
Could these not be represented as a bitset?
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:84
+
+static Expected<DynTypedNode> getNode(const BoundNodes &Nodes, StringRef ID) {
+ auto &NodesMap = Nodes.getMap();
----------------
Should there ever be a case where this returns an error. If not can this assert
instead and just return a DynTypedNode?
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:117-122
+ for (; OptToken.hasValue() && !IsConstToken(OptToken.getValue()) &&
+ !OptToken.getValue().isOneOf(tok::TokenKind::l_brace,
+ tok::TokenKind::kw_default,
+ tok::TokenKind::semi);
+ OptToken = GetNextToken(OptToken.getValue().getLocation())) {
+ }
----------------
Looks like this is begging to be a while loop.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:129-133
+ // This error is unreachable within this check. constSpec is only called
+ // on defaulted symmetric binary operators, which are required to be
const
+ // specified.
+ return invalidArgumentError(
+ "constSpec invalid argument: member function is not const
qualified");
----------------
If its unreachable, use unreachable to indicate that.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:134
+ "constSpec invalid argument: member function is not const
qualified");
+ };
+ return invalidArgumentError(
----------------
This semi-colon looks suspicious
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/SymmetricBinaryOperatorCheck.cpp:135-136
+ };
+ return invalidArgumentError(
+ "constSpec invalid argument: not a CXXMethodDecl");
+ };
----------------
Is this error reachable? Again if not use unreachable.
If it is reachable, is it possible to show a test case where this error is
expected and detected in the test?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128861/new/
https://reviews.llvm.org/D128861
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
