[PATCH] D94128: [ASTMatchers] Make cxxOperatorCallExpr matchers API-compatible with n-ary operators

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:1994
+      return None;
+    return FD->getNumParams() > 0 ? UO_PostInc : UO_PreInc;
+  }
----------------
aaron.ballman wrote:
> Not certain how much we want to care about it, but I think we need to check whether there's exactly one parameter of type `int` to meet the language rules for these to be replacement APIs: http://eel.is/c++draft/over.inc#1
> 
> e.g., the user could do something rather unlikely like: `int operator++(float);` for their overload and we're report it as being equivalent when it really isn't.
It doesn't compile with the big three at least: https://godbolt.org/z/Kns8We

So, we can't write a test for it.

So, we shouldn't add a condition for it.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:2142-2150
+  auto optBinaryOpcode = equivalentBinaryOperator(Node);
+  if (!optBinaryOpcode) {
+    auto optUnaryOpcode = equivalentUnaryOperator(Node);
+    if (!optUnaryOpcode) {
+      return false;
+    }
+    return Name == UnaryOperator::getOpcodeStr(*optUnaryOpcode);
----------------
aaron.ballman wrote:
> Can we reuse the implementation from `getOpName()` to simplify this?
Yes, good idea. I went one step further and removed this new function entirely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94128