[libcxx-commits] [PATCH] D140584: [Clang] Refactor "Designators" into a unified implementation [NFC]

[Bill Wendling via Phabricator via libcxx-commits]

void 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:
> 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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140584