rjmccall added a comment. In D72841#1917459 <https://reviews.llvm.org/D72841#1917459>, @rjmccall wrote:
> In D72841#1917340 <https://reviews.llvm.org/D72841#1917340>, @mibintc wrote: > > > @rjmccall Since CompoundAssignmentOperator derives from BinaryOperator, > > it's not simple to add Trailing storage here. I think I will have to fold > > CompoundAssignmentOperator into BinaryOperator and then add the 2 extra > > fields needed by CompoundAssignmentOperator into Trailing storage. Can you > > think of a better way? I worked on Trailing storage for UnaryOperator > > first and that wasn't too bad, but Binary is a different story. > > > It's something we deal with occasionally, but it's definitely annoying. You > basically have to test for which concrete class you have and then ask that > class for its trailing storage. > > Collapsing the types might be okay but could get involved. To be clear, we do this in the Clang AST by doing a lot of hand-rolled trailing storage rather than by using llvm::TrailingStorage. If you're willing to do the refactors necessary to use the latter, though, that's great; but as mentioned, you'll need to either collapse CompoundAssignmentOperator into BinaryOperator or introduce a concrete subclass for the non-compound operators and make BinaryOperator an abstract node. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72841/new/ https://reviews.llvm.org/D72841 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
