[PR,c++/84596] implicit conv in static assert

Message ID or7eqxpb4j.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • [PR,c++/84596] implicit conv in static assert
Related show

Commit Message

Alexandre Oliva Feb. 28, 2018, 12:11 p.m.
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


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Comments

Marek Polacek Feb. 28, 2018, 1:57 p.m. | #1
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
Jason Merrill Feb. 28, 2018, 7:01 p.m. | #2
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
Marek Polacek March 1, 2018, 4:28 p.m. | #3
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
Jason Merrill March 1, 2018, 4:57 p.m. | #4
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
Alexandre Oliva March 2, 2018, 2:38 a.m. | #5
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!

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer

Patch

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" }
+}