rjmccall added a comment.

Okay, that's fine, it's definitely a simpler change to not mess with the basic 
node hierarchy.  The one problem I see is that you've (accidentally?) set up 
the `FPOptions` methods so that you can't call them on a `BinaryOperator` 
that's actually a `CompoundAssignOperator` — they'll end up accessing the wrong 
memory.  Fortunately, that's pretty easy to fix, and fixing it should let you 
drop all the duplicate methods you've currently got on `CompoundAssignOperator`.



================
Comment at: clang/include/clang/AST/Expr.h:3480
+                reinterpret_cast<const char *>(this) + sizeof(BinaryOperator));
+    return slot;
+  }
----------------
You should add an `offsetOfTrailingStorage()` method that returns 
`isa<CompoundAssignOperator>(this) ? sizeof(CompoundAssignOperator) : 
sizeof(BinaryOperator)`, and then you can add that here instead of hard-coding 
`sizeof(BinaryOperator)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76384



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to