[PATCH] D56959: [AST] NFC: Introduce new class GenericSelectionExpr::Association

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 21 07:07:34 PST 2019


aaron.ballman added inline comments.


================
Comment at: include/clang/AST/Expr.h:5068
 
+  Association getAssociation(unsigned I) const {
+    return Association(cast<Expr>(SubExprs[END_EXPR + I]), AssocTypes[I],
----------------
steveire wrote:
> aaron.ballman wrote:
> > Rather than gin these objects up on demand every time they're needed, I'd prefer to see the class store `Association` objects directly. I don't think it will be easy to do that and still support `getAssocExprs()` and `getAssocTypeSourceInfos()`, so I think those APIs should be removed in favor of this one. There's currently not many uses of `getAssocExprs()` or `getAssocTypeSourceInfos()` (I spot one each in Clang) so migration to the new API should not be onerous.
> > 
> > This should also have a range-based accessor version so that users aren't required to use iterative loops to access the information (a lot of the places you're already touching could use that range-based interface).
> I would prefer that too, but it doesn't seem to be possible. This is a sub-range of the `SubExprs` returned from `children()`. 
> 
> In theory, that could be split into two members, but then you would need a range library to recombine them and implement `children()`: https://godbolt.org/z/ZVamdC
> 
> This seems to be the best approach for now, and AFAIK it excludes a range-accessor.
> I would prefer that too, but it doesn't seem to be possible. This is a sub-range of the SubExprs returned from children().

Ugh, you're right. :-(

> In theory, that could be split into two members, but then you would need a range library to recombine them and implement children(): https://godbolt.org/z/ZVamdC

We have zip iterators that could be used to implement this, I believe. (see STLExtras.h)

Alternatively, we could tail-allocate the Association objects with (perhaps references to) pointers back into the Expr tail-allocated array. Not ideal, but does provide a clean interface.

@riccibruno may have other ideas on how to pack the arrays, as he's done a lot of this work recently.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56959





More information about the cfe-commits mailing list