[PATCH] D99466: Fix PR48889: assertion failure for void() in flattened ternary expression

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:466-469
+    // [expr.type.conv]: if the type is cv void and the initializer is () or {},
+    // the expression is a prvalue of type void that performs no initialization.
+    QualType T = E->getType();
+    return T->isVoidType() ? nullptr : EmitNullValue(T);
----------------
rsmith wrote:
> Would it make sense to put this check in `EmitNullValue` instead? That would also cover an `InitListExpr` for `void{}` (which we currently appear to incorrectly reject), and an `ImplicitValueInit` for type `void` (which I don't think can be created legitimately at the moment, but that could just be a failure of imagination on my part).
You're right, we do reject `void{}`. Perhaps something I should address first, so that we can have tests for both cases.

I was considering putting this in `EmitNullValue`, but then I read through the standard and it seems `void` isn't value/zero-initializable. Then I found [expr.type.conv] where it says `void()` is allowed and “performs no initialization.” So if we stick close to the standard it seems we should catch the special case here, but in `EmitNullValue` it would indeed cover both `void()` and `void{}`.

Regarding `void{}`: you might know that `InitListExpr`s have `void` type initially, and there is an assertion that it doesn't have `void` type after Sema ran over it. I'd have to remove that assertion, unless we give `InitListExpr`s a different initial type. Maybe you can comment on D98664 what you think about this, where I'm trying to bring `ParenListExpr` in line with `InitListExpr` regarding the initial type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99466