rsmith added inline comments.
================
Comment at: clang/include/clang/AST/Designator.h:88
+ /// An array designator, e.g., "[42] = 0" and "[42 ... 50] = 1".
+ template <typename Ty> struct ArrayDesignatorInfo {
+ /// Location of the first and last index expression within the designated
----------------
void wrote:
> void wrote:
> > rsmith wrote:
> > > Can we move the templating out from here to the whole `Designator` and
> > > `Designation` classes? It shouldn't be possible to mix the two kinds in
> > > the same `Designation`.
> > Grr...My previous comment was eaten.
> >
> > I'll give it a shot.
> >
> > However, I'm a bit surprised at how designators are handled by Clang. I
> > expected that a `Designation` would be an `Expr` with the `Designator`s
> > being L-values (e.g. `MemberExpr`s / `ArraySubscriptExpr`s), but instead
> > the `Designation` exists just long enough to be turned into an explicit
> > initialization list. Is there a reason to do it that way instead of using
> > expressions?
> So it looks like moving the template outside of the class won't work. The
> ability to switch between `Expr` and `unsigned` while retaining the same
> overall type is hardwired into things like the `ASTImporter`.
>
> This is kind of a massive mess. Maybe we shouldn't even allow them to use
> both `Expr` and `unsigned` but instead require them to use one or the other?
> Maybe we could require `unsigned` with the understanding that the `Expr` can
> be converted into a constant?
I'm not understanding something. Currently the `ASTImporter` only deals with
`DesignatedInitExpr::Designator`s , which only ever store integer indexes.
Basically, today, we have two different classes:
- A class that's specific to `DesignatedInitExpr`, and tracks array index
expressions by storing the index of the expression within the
`DesignatedInitExpr`'s list of children; this is also what `ASTImporter` can
import, because it's the one that's used in the AST's representation.
- A class that's specific to `Sema`'s processing that tracks array index
expressions as `Expr*` instead.
You want to refactor them to share code, which makes sense, because they are
basically the same other than how they refer to expressions. (Not quite:
`DesignatedInitExpr` can apparently refer to a field either as an
`IdentifierInfo*` or as a `FieldDecl*`, whereas the `Sema` version always uses
the `IdentifierInfo*` representation.)
Each current user of one of these two classes uses only one of the two, which
means they're either exclusively using integers to refer to expressions or
exclusively using `Expr*`. So it seems to me that you should be able to update
each user to use either `Designator<unsigned>` or `Designator<Expr*>`,
depending on which class they used before.
What am I missing?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140584/new/
https://reviews.llvm.org/D140584
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
