diff mbox

[C++] Fix PR67333

Message ID 56229A1D.8080700@gmail.com
State New
Headers show

Commit Message

Mikhail Maltsev Oct. 17, 2015, 6:57 p.m. UTC 
On 10/06/2015 04:46 PM, Jason Merrill wrote:
> Hi, sorry for the slow response.  Please feel free to ping once a week.
> 
> On 08/27/2015 02:27 PM, Mikhail Maltsev wrote:
>> +  if (TREE_THIS_VOLATILE (t) && (!DECL_P (t) || want_rval))
> 
> Why the !DECL_P check?  Pulling the value out of a volatile INDIRECT_REF is just
> as problematic as from a variable.
Hmm... my intent was to check that we are not dealing with an expression, i.e.
that should have been 'EXPR_P' rather than '!DECL_P'.

I changed the condition to 'TREE_THIS_VOLATILE (t) && (EXPR_P (t) || want_rval)'
and also added one more test (the one with 'test_ref' function). The updated
patch passes bootstrap & regtest on x86_64-pc-linux-gnu.

But now I wonder, if the combination 'TREE_THIS_VOLATILE (t) && EXPR_P (t) &&
!want_rval' is possible at all and should it be rejected?

Comments

Jason Merrill Oct. 18, 2015, 8:25 a.m. UTC | #1 
On 10/17/2015 08:57 AM, Mikhail Maltsev wrote:
> On 10/06/2015 04:46 PM, Jason Merrill wrote:
>> Hi, sorry for the slow response.  Please feel free to ping once a week.
>>
>> On 08/27/2015 02:27 PM, Mikhail Maltsev wrote:
>>> +  if (TREE_THIS_VOLATILE (t) && (!DECL_P (t) || want_rval))
>>
>> Why the !DECL_P check?  Pulling the value out of a volatile INDIRECT_REF is just
>> as problematic as from a variable.

> Hmm... my intent was to check that we are not dealing with an expression, i.e.
> that should have been 'EXPR_P' rather than '!DECL_P'.

But why do you want to check that?  My point is that the same issue can 
occur with an expression.  Can't you just check want_rval?

> But now I wonder, if the combination 'TREE_THIS_VOLATILE (t) && EXPR_P (t) &&
> !want_rval' is possible at all and should it be rejected?

It is possible for a volatile *_REF.

Jason
diff mbox

Patch

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index ec9710c..b23d52a 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4006,7 +4006,7 @@  potential_constant_expression_1 (tree t, bool want_rval, bool strict,
     return false;
   if (t == NULL_TREE)
     return true;
-  if (TREE_THIS_VOLATILE (t))
+  if (TREE_THIS_VOLATILE (t) && (EXPR_P (t) || want_rval))
     {
       if (flags & tf_error)
         error ("expression %qE has side-effects", t);
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-67333.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-67333.C
new file mode 100644
index 0000000..885ece6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-67333.C
@@ -0,0 +1,29 @@ 
+// PR c++/67333
+// { dg-do compile { target c++11 } }
+
+template <int N>
+struct integral_constant
+{
+  static constexpr int value = N;
+};
+
+template <typename T, int S>
+constexpr int lengthof (const volatile T (&)[S])
+{
+  return S;
+}
+
+template <typename T, int S>
+constexpr int valueof (const volatile T (&s)[S])  // { dg-error "has side-effects" }
+{
+  return s[0];
+}
+
+int main ()
+{
+  volatile int meow[4] {};
+  integral_constant<lengthof (meow)>::value;  // OK
+  integral_constant<valueof (meow)>::value;   // { dg-error "in a constant expression" }
+  return 0;
+}
+
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-67333.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-67333.C
new file mode 100644
index 0000000..333047a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-67333.C
@@ -0,0 +1,36 @@ 
+// PR c++/67333
+// { dg-do compile { target c++14 } }
+
+template <int N>
+struct integral_constant
+{
+  static constexpr int value = N;
+};
+
+constexpr int decl (int x)
+{
+  volatile int v = x;
+  return x;
+}
+
+constexpr int use (int x)
+{
+  volatile int v = x;
+  return v;
+} // { dg-error "has side-effects" }
+
+constexpr int test_ref (volatile int &x)
+{
+  volatile int &vol_ref = x;
+  volatile int *vol_ptr = &x;
+  volatile int &vol_deref = *vol_ptr;
+  return 0;
+}
+
+int main()
+{
+  volatile int x = 0;
+  constexpr int t = test_ref (x);
+  integral_constant<decl (2)>::value; // OK
+  integral_constant<use (2)>::value;  // { dg-error "in a constant expression" }
+}