[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

Mon Dec 5 11:46:44 PST 2022

ayzhao marked 4 inline comments as done.
ayzhao added a comment.

In D129531#3971392 <https://reviews.llvm.org/D129531#3971392>, @ilya-biryukov wrote:

> Thanks!
>
> I have two major comments and also inline NITs. Not sure if we should block on those, just wanted to hear your opinions:
>
> - `InitListExpr` and `CXXParenInitListExpr` have some code in common. Code duplication is substantial and I think sharing the common implementation is warranted. E.g. if some code would want to change something with `ArrayFiller` or make a use of it, the work will probably need to be duplicated. To reiterate what I mentioned earlier, I believe deriving `CXXParenInitListExpr` from `InitListExpr` is not a very good option, but we might explore other possibilities: a base class or factoring out common code in a separate struct. I believe this could be done with a follow-up change, though, as the amount of changes required does not seem too big, I would be happy with first landing this patch and then cleaning up with a follow-up change.
> - Did we explore the possibility of modelling this differently and somehow avoiding making `CXXParenInitListExpr` an expression. This seems to borrow from `InitListExpr`, but maybe for the wrong reasons.  Braced initializers can actually occur in expressions positions, e.g. `int a = {123}`. However, `CXXParenInistListExpr` cannot occur in expression positions outside initializations, e.g. having `CXXFunctionalCastExpr` for `aggregate_struct(123)` feels somewhat wrong to me and seems to lead to confusing error messages too. Maybe we should model it as a new expression that contains both the type and the argument list? I.e. more similar to `CXXConstructExpr`?

As discussed offline, let's keep the current implementation as is for now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531