On Wed, 15 Apr 2020, Jason Merrill wrote:
> On 4/15/20 4:43 PM, Patrick Palka wrote:
> > Oops, consider the typo in the subject line fixed. Also ...
> >
> > On Wed, 15 Apr 2020, Patrick Palka wrote:
> >
> > > Here we're ICE'ing in do_narrow during error-recovery, because ocp_convert
> > > returns error_mark_node after it attempts to reduce a const decl to its
> > > erroneous DECL_INITIAL via scalar_constant_value, and we later pass this
> > > error_mark_node to fold_build2 which isn't prepared to handle
> > > error_mark_nodes.
> > >
> > > We could fix this ICE in do_narrow by checking if ocp_convert returns
> > > error_mark_node, but for the sake of consistency and for better error
> > > recovery
> > > it seems it'd be better if ocp_convert didn't care that a const decl's
> > > initializer is erroneous and would instead proceed as if the decl was not
> > > const,
> > > which is the approach that this patch takes.
> > >
> > > Passes 'make check-c++', does this look OK to commit after full bootstrap
> > > and
> > > regtest?
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > PR c++/94475
> > > * cvt.c (ocp_convert): If the result of scalar_constant_value is
> > > erroneous, discard it and carry on with the original expression.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > PR c++/94475
> > > * g++.dg/conversion/err-recover2.C: New test.
> > > * g++.dg/diagnostic/pr84138.C: Remove now-bogus warning.
> > > * g++.dg/warn/Wsign-compare-8.C: Remove now-bogus warning.
> > > ---
> > > gcc/cp/cvt.c | 6 +++---
> > > gcc/testsuite/g++.dg/conversion/err-recover2.C | 10 ++++++++++
> > > gcc/testsuite/g++.dg/diagnostic/pr84138.C | 2 +-
> > > gcc/testsuite/g++.dg/warn/Wsign-compare-8.C | 2 +-
> > > 4 files changed, 15 insertions(+), 5 deletions(-)
> > > create mode 100644 gcc/testsuite/g++.dg/conversion/err-recover2.C
> > >
> > > diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
> > > index a3b80968b33..b94231a6d08 100644
> > > --- a/gcc/cp/cvt.c
> > > +++ b/gcc/cp/cvt.c
> > > @@ -723,10 +723,10 @@ ocp_convert (tree type, tree expr, int convtype, int
> > > flags,
> > > if (!CLASS_TYPE_P (type))
> > > {
> > > e = mark_rvalue_use (e);
> > > - e = scalar_constant_value (e);
> > > + tree v = scalar_constant_value (e);
> > > + if (!error_operand_p (v))
> > > + e = v;
> > > }
> > > - if (error_operand_p (e))
> > > - return error_mark_node;
> >
> > Removing this error_operand_p check might make an error_mark_node slip
> > through and get processed by the rest of ocp_convert, if the call to
> > mark_rvalue_use above returns error_mark_node.
> >
> > In light of that, please consider this patch instead which restores that
> > error_operand_p check:
>
> OK. I wonder if we want to drop the call to scalar_constant_value entirely in
> GCC 11, and expect that the expression will be folded properly later.
That makes sense to me. On its own, removing the call entirely causes a
few testsuite regressions, but I haven't looked into any of them yet.
>
> > -- >8 --
> >
> > Subject: [PATCH] c++: Error recovery with erroneous DECL_INITIAL [PR94475]
> >
> > gcc/cp/ChangeLog:
> >
> > PR c++/94475
> > * cvt.c (ocp_convert): If the result of scalar_constant_value is
> > erroneous, ignore it and use the original expression.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/94475
> > * g++.dg/conversion/err-recover2.C: New test.
> > * g++.dg/diagnostic/pr84138.C: Remove now-bogus warning.
> > * g++.dg/warn/Wsign-compare-8.C: Remove now-bogus warning.
> > ---
> > gcc/cp/cvt.c | 4 +++-
> > gcc/testsuite/g++.dg/conversion/err-recover2.C | 10 ++++++++++
> > gcc/testsuite/g++.dg/diagnostic/pr84138.C | 2 +-
> > gcc/testsuite/g++.dg/warn/Wsign-compare-8.C | 2 +-
> > 4 files changed, 15 insertions(+), 3 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/conversion/err-recover2.C
> >
> > diff --git a/gcc/cp/cvt.c b/gcc/cp/cvt.c
> > index a3b80968b33..656e7fd3ec0 100644
> > --- a/gcc/cp/cvt.c
> > +++ b/gcc/cp/cvt.c
> > @@ -723,7 +723,9 @@ ocp_convert (tree type, tree expr, int convtype, int
> > flags,
> > if (!CLASS_TYPE_P (type))
> > {
> > e = mark_rvalue_use (e);
> > - e = scalar_constant_value (e);
> > + tree v = scalar_constant_value (e);
> > + if (!error_operand_p (v))
> > + e = v;
> > }
> > if (error_operand_p (e))
> > return error_mark_node;
> > diff --git a/gcc/testsuite/g++.dg/conversion/err-recover2.C
> > b/gcc/testsuite/g++.dg/conversion/err-recover2.C
> > new file mode 100644
> > index 00000000000..437e1a919ea
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/conversion/err-recover2.C
> > @@ -0,0 +1,10 @@
> > +// PR c++/94475
> > +// { dg-do compile }
> > +
> > +unsigned char
> > +sr ()
> > +{
> > + const unsigned char xz = EI; // { dg-error "not declared" }
> > +
> > + return xz - (xz >> 1);
> > +}
> > diff --git a/gcc/testsuite/g++.dg/diagnostic/pr84138.C
> > b/gcc/testsuite/g++.dg/diagnostic/pr84138.C
> > index 5c48b9b164a..00352306a56 100644
> > --- a/gcc/testsuite/g++.dg/diagnostic/pr84138.C
> > +++ b/gcc/testsuite/g++.dg/diagnostic/pr84138.C
> > @@ -5,4 +5,4 @@ foo()
> > {
> > const int i = 0 = 0; // { dg-error "lvalue required as left operand" }
> > return 1 ? 0 : (char)i;
> > -} // { dg-warning "control reaches" }
> > +}
> > diff --git a/gcc/testsuite/g++.dg/warn/Wsign-compare-8.C
> > b/gcc/testsuite/g++.dg/warn/Wsign-compare-8.C
> > index 237ba84d526..4d2688157fc 100644
> > --- a/gcc/testsuite/g++.dg/warn/Wsign-compare-8.C
> > +++ b/gcc/testsuite/g++.dg/warn/Wsign-compare-8.C
> > @@ -5,4 +5,4 @@ bool foo (char c)
> > {
> > const int i = 0 = 0; // { dg-error "lvalue" }
> > return c = i;
> > -} // { dg-warning "control reaches" }
> > +}
> >
>
>
