steveire marked an inline comment as done.
steveire added inline comments.
================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5346
+ AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator, CXXOperatorCallExpr,
+ CXXRewrittenBinaryOperator)) {
return Node.isAssignmentOp();
----------------
aaron.ballman wrote:
> This change surprises me -- aren't rewritten binary operators *always*
> comparison operations? I don't know of a time when they'd ever be an
> assignment operator.
Yes, which is why the new method in this patch on that class returns `true`.
This makes it possible to write
```
binaryOperation(isComparisonOperator())
binaryOperation(isAssignmentOperator())
```
(possibly with `unless`).
================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:260
+
+ auto DCF = Node->getDecomposedForm();
+ if (!match(*DCF.LHS) || !match(*DCF.RHS))
----------------
aaron.ballman wrote:
> Please spell out this use of `auto`.
Please specify what to replace auto with when the replacement is a condition of
acceptance.
Type noise makes code less readable, especially if the type contains `<`, `>`
`,`, spaces or `::` or a combination of all of them. There is also ambiguity
about what part of the type is a namespace, and what is an enclosing class etc.
Very confusing,
In C++98 typedefs were often used to make that problem less bad.
The C++11 solution is `auto`.
So, you require that an `auto` must be replaced with something less optimal,
please be specific about what you require it replaced with. I don't know what
you want here, but my guess is it is a long type which makes the code less
readable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94130/new/
https://reviews.llvm.org/D94130
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
