diff mbox series

C++ PATCH for c++/86608, reading constexpr volatile variable

Message ID 20181211014841.GQ21364@redhat.com
State New
Headers show
Series C++ PATCH for c++/86608, reading constexpr volatile variable | expand

Commit Message

Marek Polacek Dec. 11, 2018, 1:48 a.m. UTC 
A template-argument for a non-type template-parameter shall be a converted
constant expression.  But an lvalue-to-rvalue conversion applied to a volatile
glvalue is not allowed to be part of the evaluation of a constant expression.
So this test should be rejected.

The non_const_var_error tweaks are needed to give the correct diagnostic.

I don't plan to backport this to older branches, unless you think otherwise.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-12-10  Marek Polacek  <polacek@redhat.com>

	PR c++/86608 - reading constexpr volatile variable.
	* constexpr.c (non_const_var_error): Add a boolean parameter.  Use it.
	(cxx_eval_constant_expression): Adjust call to non_const_var_error.
	(potential_constant_expression_1): Also give error when reading a
	constexpr volatile variable.

	* g++.dg/cpp0x/constexpr-volatile2.C: New test.
	* g++.dg/cpp0x/pr65327.C: Add dg-error.

Comments

Jason Merrill Dec. 11, 2018, 3:26 p.m. UTC | #1 
On 12/10/18 8:48 PM, Marek Polacek wrote:
> A template-argument for a non-type template-parameter shall be a converted
> constant expression.  But an lvalue-to-rvalue conversion applied to a volatile
> glvalue is not allowed to be part of the evaluation of a constant expression.
> So this test should be rejected.

It occurred to me after my note on IRC that the 
potential_constant_expression_1 test we were talking about,

   if (TREE_THIS_VOLATILE (t) && !DECL_P (t))

ought to test want_rval rather than !DECL_P so that we consistently 
reject the lvalue-to-rvalue conversion, and not other uses of a volatile 
lvalue.  And the diagnostic ought to talk about that rather than 
"side-effects".

It might still be appropriate to change non_const_var_error, but I'd 
think it could check TREE_THIS_VOLATILE itself, rather than the caller; 
I don't see a need for the two calls to differ in their handling of 
volatile variables.

Perhaps decl_maybe_constant_var_p should return false for constexpr 
volatile, as well.

Jason
Marek Polacek Dec. 11, 2018, 6:35 p.m. UTC | #2 
On Tue, Dec 11, 2018 at 10:26:00AM -0500, Jason Merrill wrote:
> On 12/10/18 8:48 PM, Marek Polacek wrote:
> > A template-argument for a non-type template-parameter shall be a converted
> > constant expression.  But an lvalue-to-rvalue conversion applied to a volatile
> > glvalue is not allowed to be part of the evaluation of a constant expression.
> > So this test should be rejected.
> 
> It occurred to me after my note on IRC that the
> potential_constant_expression_1 test we were talking about,
> 
>   if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
> 
> ought to test want_rval rather than !DECL_P so that we consistently reject
> the lvalue-to-rvalue conversion, and not other uses of a volatile lvalue.
> And the diagnostic ought to talk about that rather than "side-effects".

Done.

> It might still be appropriate to change non_const_var_error, but I'd think
> it could check TREE_THIS_VOLATILE itself, rather than the caller; I don't
> see a need for the two calls to differ in their handling of volatile
> variables.

This change was no longer needed...

> Perhaps decl_maybe_constant_var_p should return false for constexpr
> volatile, as well.

...neither was this but I did it all the same because I think it's reasonable.

Like I said, not planning to backport it, but I could be easily convinced.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-12-11  Marek Polacek  <polacek@redhat.com>

	PR c++/86608 - reading constexpr volatile variable.
	* constexpr.c (potential_constant_expression_1): Check want_rval
	instead of checking if we have a decl.
	* decl2.c (decl_maybe_constant_var_p): Don't consider volatile
	constexpr variables as maybe constant.

	* g++.dg/cpp0x/constexpr-volatile2.C: New test.
	* g++.dg/cpp0x/pr65327.C: Add dg-error.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 1c844a8c2ef..44db38029bf 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -5476,10 +5476,11 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
        available, so we don't bother with switch tracking.  */
     return true;
 
-  if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
+  if (TREE_THIS_VOLATILE (t) && want_rval)
     {
       if (flags & tf_error)
-        error_at (loc, "expression %qE has side-effects", t);
+	error_at (loc, "lvalue-to-rvalue conversion of a volatile lvalue "
+		  "%qE with type %qT", t, TREE_TYPE (t));
       return false;
     }
   if (CONSTANT_CLASS_P (t))
diff --git gcc/cp/decl2.c gcc/cp/decl2.c
index a8bf28a0cd9..1b3e758b625 100644
--- gcc/cp/decl2.c
+++ gcc/cp/decl2.c
@@ -4313,7 +4313,7 @@ decl_maybe_constant_var_p (tree decl)
   tree type = TREE_TYPE (decl);
   if (!VAR_P (decl))
     return false;
-  if (DECL_DECLARED_CONSTEXPR_P (decl))
+  if (DECL_DECLARED_CONSTEXPR_P (decl) && !TREE_THIS_VOLATILE (decl))
     return true;
   if (DECL_HAS_VALUE_EXPR_P (decl))
     /* A proxy isn't constant.  */
diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
new file mode 100644
index 00000000000..0def8d73731
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
@@ -0,0 +1,13 @@
+// PR c++/86608
+// { dg-do compile { target c++11 } }
+
+template<typename T, T v> struct X {};
+
+int
+main ()
+{
+  static constexpr volatile int a = 3;
+  constexpr volatile int b = 2;
+  return (sizeof(X<decltype(a), a>) // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }
+	  + sizeof(X<decltype(b), b>)); // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }
+}
diff --git gcc/testsuite/g++.dg/cpp0x/pr65327.C gcc/testsuite/g++.dg/cpp0x/pr65327.C
index c6cefaba692..5176b3c3204 100644
--- gcc/testsuite/g++.dg/cpp0x/pr65327.C
+++ gcc/testsuite/g++.dg/cpp0x/pr65327.C
@@ -15,4 +15,4 @@ constexpr volatile int
 bar ()
 {
   return i;
-}
+} // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }
Jason Merrill Dec. 11, 2018, 6:48 p.m. UTC | #3 
OK.
On Tue, Dec 11, 2018 at 1:35 PM Marek Polacek <polacek@redhat.com> wrote:
>
> On Tue, Dec 11, 2018 at 10:26:00AM -0500, Jason Merrill wrote:
> > On 12/10/18 8:48 PM, Marek Polacek wrote:
> > > A template-argument for a non-type template-parameter shall be a converted
> > > constant expression.  But an lvalue-to-rvalue conversion applied to a volatile
> > > glvalue is not allowed to be part of the evaluation of a constant expression.
> > > So this test should be rejected.
> >
> > It occurred to me after my note on IRC that the
> > potential_constant_expression_1 test we were talking about,
> >
> >   if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
> >
> > ought to test want_rval rather than !DECL_P so that we consistently reject
> > the lvalue-to-rvalue conversion, and not other uses of a volatile lvalue.
> > And the diagnostic ought to talk about that rather than "side-effects".
>
> Done.
>
> > It might still be appropriate to change non_const_var_error, but I'd think
> > it could check TREE_THIS_VOLATILE itself, rather than the caller; I don't
> > see a need for the two calls to differ in their handling of volatile
> > variables.
>
> This change was no longer needed...
>
> > Perhaps decl_maybe_constant_var_p should return false for constexpr
> > volatile, as well.
>
> ...neither was this but I did it all the same because I think it's reasonable.
>
> Like I said, not planning to backport it, but I could be easily convinced.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-12-11  Marek Polacek  <polacek@redhat.com>
>
>         PR c++/86608 - reading constexpr volatile variable.
>         * constexpr.c (potential_constant_expression_1): Check want_rval
>         instead of checking if we have a decl.
>         * decl2.c (decl_maybe_constant_var_p): Don't consider volatile
>         constexpr variables as maybe constant.
>
>         * g++.dg/cpp0x/constexpr-volatile2.C: New test.
>         * g++.dg/cpp0x/pr65327.C: Add dg-error.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 1c844a8c2ef..44db38029bf 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -5476,10 +5476,11 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
>         available, so we don't bother with switch tracking.  */
>      return true;
>
> -  if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
> +  if (TREE_THIS_VOLATILE (t) && want_rval)
>      {
>        if (flags & tf_error)
> -        error_at (loc, "expression %qE has side-effects", t);
> +       error_at (loc, "lvalue-to-rvalue conversion of a volatile lvalue "
> +                 "%qE with type %qT", t, TREE_TYPE (t));
>        return false;
>      }
>    if (CONSTANT_CLASS_P (t))
> diff --git gcc/cp/decl2.c gcc/cp/decl2.c
> index a8bf28a0cd9..1b3e758b625 100644
> --- gcc/cp/decl2.c
> +++ gcc/cp/decl2.c
> @@ -4313,7 +4313,7 @@ decl_maybe_constant_var_p (tree decl)
>    tree type = TREE_TYPE (decl);
>    if (!VAR_P (decl))
>      return false;
> -  if (DECL_DECLARED_CONSTEXPR_P (decl))
> +  if (DECL_DECLARED_CONSTEXPR_P (decl) && !TREE_THIS_VOLATILE (decl))
>      return true;
>    if (DECL_HAS_VALUE_EXPR_P (decl))
>      /* A proxy isn't constant.  */
> diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
> new file mode 100644
> index 00000000000..0def8d73731
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
> @@ -0,0 +1,13 @@
> +// PR c++/86608
> +// { dg-do compile { target c++11 } }
> +
> +template<typename T, T v> struct X {};
> +
> +int
> +main ()
> +{
> +  static constexpr volatile int a = 3;
> +  constexpr volatile int b = 2;
> +  return (sizeof(X<decltype(a), a>) // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }
> +         + sizeof(X<decltype(b), b>)); // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }
> +}
> diff --git gcc/testsuite/g++.dg/cpp0x/pr65327.C gcc/testsuite/g++.dg/cpp0x/pr65327.C
> index c6cefaba692..5176b3c3204 100644
> --- gcc/testsuite/g++.dg/cpp0x/pr65327.C
> +++ gcc/testsuite/g++.dg/cpp0x/pr65327.C
> @@ -15,4 +15,4 @@ constexpr volatile int
>  bar ()
>  {
>    return i;
> -}
> +} // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }
diff mbox series

Patch

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 1c844a8c2ef..e78cc4a81d6 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -3431,10 +3431,11 @@  cxx_eval_indirect_ref (const constexpr_ctx *ctx, tree t,
 
 /* Complain about R, a VAR_DECL, not being usable in a constant expression.
    Shared between potential_constant_expression and
-   cxx_eval_constant_expression.  */
+   cxx_eval_constant_expression.  If SKIP_OWN_INIT_P is true, don't report
+   that R was used in its own initializer, as the problem is elsewhere.  */
 
 static void
-non_const_var_error (tree r)
+non_const_var_error (tree r, bool skip_own_init_p)
 {
   tree type = TREE_TYPE (r);
   error ("the value of %qD is not usable in a constant "
@@ -3442,7 +3443,7 @@  non_const_var_error (tree r)
   /* Avoid error cascade.  */
   if (DECL_INITIAL (r) == error_mark_node)
     return;
-  if (DECL_DECLARED_CONSTEXPR_P (r))
+  if (!skip_own_init_p && DECL_DECLARED_CONSTEXPR_P (r))
     inform (DECL_SOURCE_LOCATION (r),
 	    "%qD used in its own initializer", r);
   else if (INTEGRAL_OR_ENUMERATION_TYPE_P (type))
@@ -4251,7 +4252,7 @@  cxx_eval_constant_expression (const constexpr_ctx *ctx, tree t,
       if (DECL_P (r))
 	{
 	  if (!ctx->quiet)
-	    non_const_var_error (r);
+	    non_const_var_error (r, /*skip_own_init_p=*/false);
 	  *non_constant_p = true;
 	}
       break;
@@ -5692,7 +5693,7 @@  potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
       if (want_rval
 	  && !var_in_maybe_constexpr_fn (t)
 	  && !type_dependent_expression_p (t)
-	  && !decl_maybe_constant_var_p (t)
+	  && (!decl_maybe_constant_var_p (t) || TREE_THIS_VOLATILE (t))
 	  && (strict
 	      || !CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (t))
 	      || (DECL_INITIAL (t)
@@ -5701,7 +5702,7 @@  potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
 	  && !is_really_empty_class (TREE_TYPE (t)))
         {
           if (flags & tf_error)
-            non_const_var_error (t);
+	    non_const_var_error (t, TREE_THIS_VOLATILE (t));
           return false;
         }
       return true;
diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
new file mode 100644
index 00000000000..2054fb83768
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
@@ -0,0 +1,13 @@ 
+// PR c++/86608
+// { dg-do compile { target c++11 } }
+
+template<typename T, T v> struct X {};
+
+int
+main ()
+{
+  static constexpr volatile int a = 3;
+  constexpr volatile int b = 2;
+  return (sizeof(X<decltype(a), a>) // { dg-error "not usable in a constant expression" }
+	  + sizeof(X<decltype(b), b>)); // { dg-error "not usable in a constant expression" }
+}
diff --git gcc/testsuite/g++.dg/cpp0x/pr65327.C gcc/testsuite/g++.dg/cpp0x/pr65327.C
index c6cefaba692..0ba189a24b8 100644
--- gcc/testsuite/g++.dg/cpp0x/pr65327.C
+++ gcc/testsuite/g++.dg/cpp0x/pr65327.C
@@ -15,4 +15,4 @@  constexpr volatile int
 bar ()
 {
   return i;
-}
+} // { dg-error "not usable in a constant expression" }