steveire marked an inline comment as done.
steveire 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],
----------------
aaron.ballman wrote:
> 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.
> We have zip iterators that could be used to implement this, I believe.
You're right, there is a `concat` there, but on second thought - because
Association and Stmt don't share a base, I don't think it can work.
> 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.
Perhaps this can work, but I don't know how to do it. If you have scope for it
in your part of the efforts, it would be a good way to get this unblocked.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56959/new/
https://reviews.llvm.org/D56959
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
