[PATCH] D82937: Fix `isInfixBinaryOp` that returned true for postfix ++

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.


================
Comment at: clang/unittests/Tooling/CXXOperatorCallExprTest.cpp:21
+  const std::string Code = R"cpp(
+  struct X{
+    friend X operator+(X, X);
----------------
Please add a space before "{".


================
Comment at: clang/unittests/Tooling/CXXOperatorCallExprTest.cpp:24
+  };
+  void test(X x){
+    x + x;
----------------
Please add a space before "{". (Throughout the patch.)


================
Comment at: clang/unittests/Tooling/CXXOperatorCallExprTest.cpp:1
+//===- unittests/Tooling/CXXOperatorCallExprTest.cpp ----------------------===//
+//
----------------
eduucaldas wrote:
> This file is in the `unittests/Tooling` instead in the `unittests/AST` directory because I wanted to have access to the `TestVisitor` infrastructure. I know the solution is not optimal and I am looking for suggestions
Could you make a separate patch where you move `TestVisitor.h` under `clang/include/clang/Testing`? Then you can place this test into `unittests/AST`.

OTOH, since the test is not testing the visitation, WDYT about changing the test to use AST matchers instead? It would seem to lead to more straightforward tests: get the AST node, act on it. See `clang/unittests/AST/MatchVerifier.h`.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82937/new/

https://reviews.llvm.org/D82937