Message ID | or7eqxpb4j.fsf@lxoliva.fsfla.org |
---|---|
State | New |
Headers | show |
Series | [PR,c++/84596] implicit conv in static assert | expand |
On Wed, Feb 28, 2018 at 09:11:24AM -0300, Alexandre Oliva wrote: > Evaluation of constant expressions, such as those passed to > static_assert, ICEd when encountering IMPLICIT_CONV_EXPRs. > > Handle them like CONVERT_EXPR and NOP_EXPR. > > Regstrapped on x86_64- and i686-linux-gnu. Ok to install? > > for gcc/cp/ChangeLog > > PR c++/84596 > * constexpr.c (cxx_eval_constant_expression): Handle > IMPLICIT_CONV_EXPR. > > for gcc/testsuite/ChangeLog > > PR c++/84596 > * g++.dg/cpp0x/pr84596.C: New. > --- > gcc/cp/constexpr.c | 1 + > gcc/testsuite/g++.dg/cpp0x/pr84596.C | 7 +++++++ > 2 files changed, 8 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr84596.C > > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > index 4bbdbf434877..d38e2d83ae8c 100644 > --- a/gcc/cp/constexpr.c > +++ b/gcc/cp/constexpr.c > @@ -4549,6 +4549,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, > non_constant_p, overflow_p); > break; > > + case IMPLICIT_CONV_EXPR: > case CONVERT_EXPR: > case VIEW_CONVERT_EXPR: > case NOP_EXPR: I don't think it's correct to handle template codes here; they shouldn't have gotten to cxx_eval_constant_expression in the first place. Usually they're substituted in instantiate_non_dependent_expr_internal e.g. via calling fold_non_dependent_expr. So I guess we need to find out how IMPLICIT_CONV_EXPR has gotten there. Marek
On Wed, Feb 28, 2018 at 8:57 AM, Marek Polacek <polacek@redhat.com> wrote: > On Wed, Feb 28, 2018 at 09:11:24AM -0300, Alexandre Oliva wrote: >> Evaluation of constant expressions, such as those passed to >> static_assert, ICEd when encountering IMPLICIT_CONV_EXPRs. >> >> Handle them like CONVERT_EXPR and NOP_EXPR. >> >> Regstrapped on x86_64- and i686-linux-gnu. Ok to install? >> >> for gcc/cp/ChangeLog >> >> PR c++/84596 >> * constexpr.c (cxx_eval_constant_expression): Handle >> IMPLICIT_CONV_EXPR. >> >> for gcc/testsuite/ChangeLog >> >> PR c++/84596 >> * g++.dg/cpp0x/pr84596.C: New. >> --- >> gcc/cp/constexpr.c | 1 + >> gcc/testsuite/g++.dg/cpp0x/pr84596.C | 7 +++++++ >> 2 files changed, 8 insertions(+) >> create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr84596.C >> >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >> index 4bbdbf434877..d38e2d83ae8c 100644 >> --- a/gcc/cp/constexpr.c >> +++ b/gcc/cp/constexpr.c >> @@ -4549,6 +4549,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, >> non_constant_p, overflow_p); >> break; >> >> + case IMPLICIT_CONV_EXPR: >> case CONVERT_EXPR: >> case VIEW_CONVERT_EXPR: >> case NOP_EXPR: > > I don't think it's correct to handle template codes here; they shouldn't > have gotten to cxx_eval_constant_expression in the first place. Usually > they're substituted in instantiate_non_dependent_expr_internal e.g. via > calling fold_non_dependent_expr. So I guess we need to find out how > IMPLICIT_CONV_EXPR has gotten there. Agreed. Jason
On Wed, Feb 28, 2018 at 02:01:06PM -0500, Jason Merrill wrote: > On Wed, Feb 28, 2018 at 8:57 AM, Marek Polacek <polacek@redhat.com> wrote: > > On Wed, Feb 28, 2018 at 09:11:24AM -0300, Alexandre Oliva wrote: > >> Evaluation of constant expressions, such as those passed to > >> static_assert, ICEd when encountering IMPLICIT_CONV_EXPRs. > >> > >> Handle them like CONVERT_EXPR and NOP_EXPR. > >> > >> Regstrapped on x86_64- and i686-linux-gnu. Ok to install? > >> > >> for gcc/cp/ChangeLog > >> > >> PR c++/84596 > >> * constexpr.c (cxx_eval_constant_expression): Handle > >> IMPLICIT_CONV_EXPR. > >> > >> for gcc/testsuite/ChangeLog > >> > >> PR c++/84596 > >> * g++.dg/cpp0x/pr84596.C: New. > >> --- > >> gcc/cp/constexpr.c | 1 + > >> gcc/testsuite/g++.dg/cpp0x/pr84596.C | 7 +++++++ > >> 2 files changed, 8 insertions(+) > >> create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr84596.C > >> > >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c > >> index 4bbdbf434877..d38e2d83ae8c 100644 > >> --- a/gcc/cp/constexpr.c > >> +++ b/gcc/cp/constexpr.c > >> @@ -4549,6 +4549,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, > >> non_constant_p, overflow_p); > >> break; > >> > >> + case IMPLICIT_CONV_EXPR: > >> case CONVERT_EXPR: > >> case VIEW_CONVERT_EXPR: > >> case NOP_EXPR: > > > > I don't think it's correct to handle template codes here; they shouldn't > > have gotten to cxx_eval_constant_expression in the first place. Usually > > they're substituted in instantiate_non_dependent_expr_internal e.g. via > > calling fold_non_dependent_expr. So I guess we need to find out how > > IMPLICIT_CONV_EXPR has gotten there. > > Agreed. Here's my take on this; Alex, sorry for interfering. The problem is that perform_implicit_conversion_flags in finish_static_assert produces "implicit_conv_expr <c>" where "c" is a PARM_DECL. Subsequent fold_non_dependent_expr is supposed to substitute that implicit_conv_expr but doesn't because the expression is not is_nondependent_constant_expression -- a PARM_DECL is only considered a potentially constant expression if NOW is false, which it isn't. But the require_potential_rvalue_constant_expression call that follows uses NOW = false. So I think we need the following. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-03-01 Marek Polacek <polacek@redhat.com> PR c++/84596 * constexpr.c (require_rvalue_constant_expression): New function. * cp-tree.h: Declare it. * semantics.c (finish_static_assert): Use it instead of require_potential_rvalue_constant_expression. * g++.dg/cpp0x/static_assert14.C: New test. diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 4bbdbf43487..39e6cdfb33d 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -6047,7 +6047,8 @@ potential_rvalue_constant_expression (tree t) bool require_potential_constant_expression (tree t) { - return potential_constant_expression_1 (t, false, true, false, tf_warning_or_error); + return potential_constant_expression_1 (t, false, true, false, + tf_warning_or_error); } /* Cross product of the above. */ @@ -6055,7 +6056,17 @@ require_potential_constant_expression (tree t) bool require_potential_rvalue_constant_expression (tree t) { - return potential_constant_expression_1 (t, true, true, false, tf_warning_or_error); + return potential_constant_expression_1 (t, true, true, false, + tf_warning_or_error); +} + +/* Like above, but don't consider PARM_DECL a potential_constant_expression. */ + +bool +require_rvalue_constant_expression (tree t) +{ + return potential_constant_expression_1 (t, true, true, true, + tf_warning_or_error); } /* Like potential_constant_expression, but don't consider possible constexpr diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h index 743dd340245..17d8c6d2650 100644 --- gcc/cp/cp-tree.h +++ gcc/cp/cp-tree.h @@ -7409,6 +7409,7 @@ extern bool is_static_init_expression (tree); extern bool potential_rvalue_constant_expression (tree); extern bool require_potential_constant_expression (tree); extern bool require_constant_expression (tree); +extern bool require_rvalue_constant_expression (tree); extern bool require_potential_rvalue_constant_expression (tree); extern tree cxx_constant_value (tree, tree = NULL_TREE); extern tree cxx_constant_init (tree, tree = NULL_TREE); diff --git gcc/cp/semantics.c gcc/cp/semantics.c index 35569d0cb0d..87c5c669a55 100644 --- gcc/cp/semantics.c +++ gcc/cp/semantics.c @@ -8671,7 +8671,7 @@ finish_static_assert (tree condition, tree message, location_t location, else if (condition && condition != error_mark_node) { error ("non-constant condition for static assertion"); - if (require_potential_rvalue_constant_expression (condition)) + if (require_rvalue_constant_expression (condition)) cxx_constant_value (condition); } input_location = saved_loc; diff --git gcc/testsuite/g++.dg/cpp0x/static_assert14.C gcc/testsuite/g++.dg/cpp0x/static_assert14.C index e69de29bb2d..ee709f4200f 100644 --- gcc/testsuite/g++.dg/cpp0x/static_assert14.C +++ gcc/testsuite/g++.dg/cpp0x/static_assert14.C @@ -0,0 +1,7 @@ +// PR c++/84596 +// { dg-do compile { target c++11 } } + +template<int x> +void b(int c) { + static_assert (c, "c"); // { dg-error "non-constant|not a constant" } +} Marek
On Thu, Mar 1, 2018 at 11:28 AM, Marek Polacek <polacek@redhat.com> wrote: > On Wed, Feb 28, 2018 at 02:01:06PM -0500, Jason Merrill wrote: >> On Wed, Feb 28, 2018 at 8:57 AM, Marek Polacek <polacek@redhat.com> wrote: >> > On Wed, Feb 28, 2018 at 09:11:24AM -0300, Alexandre Oliva wrote: >> >> Evaluation of constant expressions, such as those passed to >> >> static_assert, ICEd when encountering IMPLICIT_CONV_EXPRs. >> >> >> >> Handle them like CONVERT_EXPR and NOP_EXPR. >> >> >> >> Regstrapped on x86_64- and i686-linux-gnu. Ok to install? >> >> >> >> for gcc/cp/ChangeLog >> >> >> >> PR c++/84596 >> >> * constexpr.c (cxx_eval_constant_expression): Handle >> >> IMPLICIT_CONV_EXPR. >> >> >> >> for gcc/testsuite/ChangeLog >> >> >> >> PR c++/84596 >> >> * g++.dg/cpp0x/pr84596.C: New. >> >> --- >> >> gcc/cp/constexpr.c | 1 + >> >> gcc/testsuite/g++.dg/cpp0x/pr84596.C | 7 +++++++ >> >> 2 files changed, 8 insertions(+) >> >> create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr84596.C >> >> >> >> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c >> >> index 4bbdbf434877..d38e2d83ae8c 100644 >> >> --- a/gcc/cp/constexpr.c >> >> +++ b/gcc/cp/constexpr.c >> >> @@ -4549,6 +4549,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, >> >> non_constant_p, overflow_p); >> >> break; >> >> >> >> + case IMPLICIT_CONV_EXPR: >> >> case CONVERT_EXPR: >> >> case VIEW_CONVERT_EXPR: >> >> case NOP_EXPR: >> > >> > I don't think it's correct to handle template codes here; they shouldn't >> > have gotten to cxx_eval_constant_expression in the first place. Usually >> > they're substituted in instantiate_non_dependent_expr_internal e.g. via >> > calling fold_non_dependent_expr. So I guess we need to find out how >> > IMPLICIT_CONV_EXPR has gotten there. >> >> Agreed. > > Here's my take on this; Alex, sorry for interfering. > > The problem is that perform_implicit_conversion_flags in finish_static_assert > produces "implicit_conv_expr <c>" where "c" is a PARM_DECL. Subsequent > fold_non_dependent_expr is supposed to substitute that implicit_conv_expr but > doesn't because the expression is not is_nondependent_constant_expression -- > a PARM_DECL is only considered a potentially constant expression if NOW is > false, which it isn't. But the require_potential_rvalue_constant_expression > call that follows uses NOW = false. So I think we need the following. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-03-01 Marek Polacek <polacek@redhat.com> > > PR c++/84596 > * constexpr.c (require_rvalue_constant_expression): New function. > * cp-tree.h: Declare it. > * semantics.c (finish_static_assert): Use it instead of > require_potential_rvalue_constant_expression. > > * g++.dg/cpp0x/static_assert14.C: New test. OK. Jason
On Mar 1, 2018, Marek Polacek <polacek@redhat.com> wrote:
> Here's my take on this; Alex, sorry for interfering.
No worries, thanks a lot for taking care of this!
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index 4bbdbf434877..d38e2d83ae8c 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -4549,6 +4549,7 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t, non_constant_p, overflow_p); break; + case IMPLICIT_CONV_EXPR: case CONVERT_EXPR: case VIEW_CONVERT_EXPR: case NOP_EXPR: diff --git a/gcc/testsuite/g++.dg/cpp0x/pr84596.C b/gcc/testsuite/g++.dg/cpp0x/pr84596.C new file mode 100644 index 000000000000..ee709f4200fe --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/pr84596.C @@ -0,0 +1,7 @@ +// PR c++/84596 +// { dg-do compile { target c++11 } } + +template<int x> +void b(int c) { + static_assert (c, "c"); // { dg-error "non-constant|not a constant" } +}