On Mon, Apr 20, 2020 at 06:37:08PM -0400, Jason Merrill wrote:
> On 4/20/20 4:44 PM, Marek Polacek wrote:
> > On Mon, Apr 20, 2020 at 11:51:59AM -0400, Jason Merrill wrote:
> > > On 4/17/20 4:28 PM, Marek Polacek wrote:
> > > > As an extension (there should be a CWG about this though), we support
> > > > braced-init-list as a template argument, but convert_nontype_argument
> > > > had trouble digesting them. We ICEd because of the double coercion we
> > > > perform for template arguments: convert_nontype_argument called from
> > > > finish_template_type got a { }, and since a class type was involved and
> > > > we were in a template, convert_like created an IMPLICIT_CONV_EXPR. Then
> > > > the second conversion of the same argument crashed in constexpr.c
> > > > because the IMPLICIT_CONV_EXPR had gotten wrapped in a TARGET_EXPR.
> > > > Another issue was that an IMPLICIT_CONV_EXPR leaked to constexpr.c when
> > > > building an aggregate init.
> > > >
> > > > We should have instantiated the IMPLICIT_CONV_EXPR in the first call to
> > > > convert_nontype_argument, but we didn't, because the call to
> > > > is_nondependent_constant_expression returned false because it checks
> > > > !BRACE_ENCLOSED_INITIALIZER_P. Then non_dep was false even though the
> > > > expression didn't contain anything dependent and we didn't instantiate
> > > > it in convert_nontype_argument. I'm not sure what the point
> > > > of that BRACE_ENCLOSED_INITIALIZER_P. check was, but removing it doesn't
> > > > break anything and fixes these crashes.
> > >
> > > The point was to avoid trying to get a constant value for an untyped
> > > braced-init-list. I'm happy to remove the check from these functions, but
> > > let's add it back to maybe_constant_{value,init}, where it was until
> > > r6-7825-geb07f187a471f9a203626aecced17d6947c3cc46 .
> >
> > Thanks for the pointer.
> >
> > > Or better, put it in cxx_eval_outermost_constant_expr.
> >
> > Done here. Since we didn't set non_constant_p and emit an error, I'm not
> > doing it here, either.
>
> Hmm, maybe checking_assert allow_non_constant to make sure we don't need an
> error. OK with that change.
Thanks; pushed after having been bootstrapped/regtested on x86_64-pc-linux-gnu.
--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
