[PATCH] D111669: No longer crash when a consteval function returns a structure

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D111669#3088970 <https://reviews.llvm.org/D111669#3088970>, @rjmccall wrote:

> Hmm.  Generally these cases are expected to handle the situation where there's no result slot passed in, which currently isn't exclusive to an ignored result (although you could argue it should be).  IIRC we usually don't pass in a slot when evaluating an expression of agg type as anything except an initializer, e.g. when evaluating the `E` in `E.member`.  The general fix is to call `EnsureDest` before accessing `Dest`.  The only reason we wouldn't need to do that is if `ConstantExpr` is restricted in where it can appear such that that's impossible.

Thank you for the suggestion to use `EnsureDest`, that worked like a charm.

> Skipping an unnecessary initialization does sound like a good optimization in general, but that's on top of the bug fix, not an alternative.
>
> Separately, seeing this code makes me worried that it doesn't support the full set of constant initialization.  We do have some kinds of constant initialization that can't be emitted as abstract initializers, e.g. address-diversified pointer auth.  But we can deal with that as a separate patch.

FWIW, there are actually quite a few crashes during codegen involving consteval support (https://bugs.llvm.org/buglist.cgi?quicksearch=consteval&list_id=224889 has quite a few of them), so I share your worries.


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

https://reviews.llvm.org/D111669