[PATCH] D76384: Move FPFeatures from BinaryOperator bitfields to Trailing storage

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 10 18:19:45 PDT 2020


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





More information about the cfe-commits mailing list