C++ PATCH for c++/91548 - fix detecting modifying const objects for ARRAY_REF

Message ID 20191023181002.GL2922@redhat.com
State New
Headers show
Series
  • C++ PATCH for c++/91548 - fix detecting modifying const objects for ARRAY_REF
Related show

Commit Message

Marek Polacek Oct. 23, 2019, 6:10 p.m.
This fixes a bogus "modifying a const object" error for an array that actually
isn't declared const.  The problem was how I handled ARRAY_REFs here; we
shouldn't look at the ARRAY_REF itself, but at the array its accessing.

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

2019-10-23  Marek Polacek  <polacek@redhat.com>

	PR c++/91548 - fix detecting modifying const objects for ARRAY_REF.
	* constexpr.c (cxx_eval_store_expression): Don't call
	modifying_const_object_p for ARRAY_REF.

	* g++.dg/cpp1y/constexpr-tracking-const15.C: New test.
	* g++.dg/cpp1y/constexpr-tracking-const16.C: New test.
	* g++.dg/cpp1z/constexpr-tracking-const1.C: New test.

Comments

Jason Merrill Oct. 29, 2019, 8:27 p.m. | #1
On 10/23/19 2:10 PM, Marek Polacek wrote:
> This fixes a bogus "modifying a const object" error for an array that actually

> isn't declared const.  The problem was how I handled ARRAY_REFs here; we

> shouldn't look at the ARRAY_REF itself, but at the array its accessing.

> 

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


OK.

> 2019-10-23  Marek Polacek  <polacek@redhat.com>

> 

> 	PR c++/91548 - fix detecting modifying const objects for ARRAY_REF.

> 	* constexpr.c (cxx_eval_store_expression): Don't call

> 	modifying_const_object_p for ARRAY_REF.

> 

> 	* g++.dg/cpp1y/constexpr-tracking-const15.C: New test.

> 	* g++.dg/cpp1y/constexpr-tracking-const16.C: New test.

> 	* g++.dg/cpp1z/constexpr-tracking-const1.C: New test.

> 

> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c

> index 11a1eaa0e82..6b4e854e35c 100644

> --- gcc/cp/constexpr.c

> +++ gcc/cp/constexpr.c

> @@ -3910,10 +3910,6 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,

>   	    tree elt = TREE_OPERAND (probe, 1);

>   	    if (TREE_CODE (elt) == FIELD_DECL && DECL_MUTABLE_P (elt))

>   	      mutable_p = true;

> -	    if (evaluated

> -		&& modifying_const_object_p (TREE_CODE (t), probe, mutable_p)

> -		&& const_object_being_modified == NULL_TREE)

> -	      const_object_being_modified = probe;

>   	    if (TREE_CODE (probe) == ARRAY_REF)

>   	      {

>   		elt = eval_and_check_array_index (ctx, probe, false,

> @@ -3921,6 +3917,15 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,

>   		if (*non_constant_p)

>   		  return t;

>   	      }

> +	    /* We don't check modifying_const_object_p for ARRAY_REFs.  Given

> +	       "int a[10]", an ARRAY_REF "a[2]" can be "const int", even though

> +	       the array isn't const.  Instead, check "a" in the next iteration;

> +	       that will detect modifying "const int a[10]".  */

> +	    else if (evaluated

> +		     && modifying_const_object_p (TREE_CODE (t), probe,

> +						  mutable_p)

> +		     && const_object_being_modified == NULL_TREE)

> +	      const_object_being_modified = probe;

>   	    vec_safe_push (refs, elt);

>   	    vec_safe_push (refs, TREE_TYPE (probe));

>   	    probe = ob;

> diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const15.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const15.C

> new file mode 100644

> index 00000000000..db1b2bb7ea6

> --- /dev/null

> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const15.C

> @@ -0,0 +1,21 @@

> +// PR c++/91548 - fix detecting modifying const objects for ARRAY_REF.

> +// { dg-do compile { target c++14 } }

> +

> +constexpr int& impl(const int (&array)[10], int index) {

> +  return const_cast<int&>(array[index]);

> +}

> +

> +struct A {

> +  constexpr int& operator[](int i) { return impl(elems, i); }

> +  int elems[10];

> +};

> +

> +constexpr bool

> +f()

> +{

> +  A arr = {};

> +  arr[2] = true;

> +  return false;

> +}

> +

> +constexpr bool b = f();

> diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const16.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const16.C

> new file mode 100644

> index 00000000000..5a5b92bc8cc

> --- /dev/null

> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const16.C

> @@ -0,0 +1,22 @@

> +// PR c++/91548 - fix detecting modifying const objects for ARRAY_REF.

> +// { dg-do compile { target c++14 } }

> +

> +constexpr int& impl(const int (&array)[10], int index) {

> +  return const_cast<int&>(array[index]);

> +}

> +

> +struct A {

> +  constexpr int& operator[](int i) { return impl(elems, i); }

> +  const int elems[10];

> +};

> +

> +constexpr bool

> +f()

> +{

> +  A arr = {};

> +  arr[2] = 1; // { dg-error "modifying a const object" }

> +  return false;

> +}

> +

> +constexpr bool b = f(); // { dg-message "in .constexpr. expansion of " }

> +// { dg-message "originally declared" "" { target *-*-* } .-1 }

> diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-tracking-const1.C gcc/testsuite/g++.dg/cpp1z/constexpr-tracking-const1.C

> new file mode 100644

> index 00000000000..a3856b8e7ec

> --- /dev/null

> +++ gcc/testsuite/g++.dg/cpp1z/constexpr-tracking-const1.C

> @@ -0,0 +1,25 @@

> +// PR c++/91548 - fix detecting modifying const objects for ARRAY_REF.

> +// { dg-do compile { target c++17 } }

> +

> +using size_t = decltype(sizeof(0));

> +

> +template <typename T, size_t N>

> +constexpr T& impl(T const (&array)[N], size_t index) {

> +    return const_cast<T&>(array[index]);

> +}

> +

> +template <typename T, size_t N>

> +struct my_array {

> +    constexpr T& operator[](size_t i) { return impl(elems, i); }

> +    constexpr T const& operator[](size_t i) const { return elems[i]; }

> +    T elems[N];

> +};

> +

> +bool f(int i) {

> +    static constexpr auto table = []() {

> +        my_array<bool, 256> arr = {};

> +        arr[2] = true;

> +        return arr;

> +    }();

> +    return table[i];

> +}

>

Patch

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 11a1eaa0e82..6b4e854e35c 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -3910,10 +3910,6 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 	    tree elt = TREE_OPERAND (probe, 1);
 	    if (TREE_CODE (elt) == FIELD_DECL && DECL_MUTABLE_P (elt))
 	      mutable_p = true;
-	    if (evaluated
-		&& modifying_const_object_p (TREE_CODE (t), probe, mutable_p)
-		&& const_object_being_modified == NULL_TREE)
-	      const_object_being_modified = probe;
 	    if (TREE_CODE (probe) == ARRAY_REF)
 	      {
 		elt = eval_and_check_array_index (ctx, probe, false,
@@ -3921,6 +3917,15 @@  cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
 		if (*non_constant_p)
 		  return t;
 	      }
+	    /* We don't check modifying_const_object_p for ARRAY_REFs.  Given
+	       "int a[10]", an ARRAY_REF "a[2]" can be "const int", even though
+	       the array isn't const.  Instead, check "a" in the next iteration;
+	       that will detect modifying "const int a[10]".  */
+	    else if (evaluated
+		     && modifying_const_object_p (TREE_CODE (t), probe,
+						  mutable_p)
+		     && const_object_being_modified == NULL_TREE)
+	      const_object_being_modified = probe;
 	    vec_safe_push (refs, elt);
 	    vec_safe_push (refs, TREE_TYPE (probe));
 	    probe = ob;
diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const15.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const15.C
new file mode 100644
index 00000000000..db1b2bb7ea6
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const15.C
@@ -0,0 +1,21 @@ 
+// PR c++/91548 - fix detecting modifying const objects for ARRAY_REF.
+// { dg-do compile { target c++14 } }
+
+constexpr int& impl(const int (&array)[10], int index) {
+  return const_cast<int&>(array[index]);
+}
+
+struct A {
+  constexpr int& operator[](int i) { return impl(elems, i); }
+  int elems[10];
+};
+
+constexpr bool
+f()
+{
+  A arr = {};
+  arr[2] = true;
+  return false;
+}
+
+constexpr bool b = f();
diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const16.C gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const16.C
new file mode 100644
index 00000000000..5a5b92bc8cc
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const16.C
@@ -0,0 +1,22 @@ 
+// PR c++/91548 - fix detecting modifying const objects for ARRAY_REF.
+// { dg-do compile { target c++14 } }
+
+constexpr int& impl(const int (&array)[10], int index) {
+  return const_cast<int&>(array[index]);
+}
+
+struct A {
+  constexpr int& operator[](int i) { return impl(elems, i); }
+  const int elems[10];
+};
+
+constexpr bool
+f()
+{
+  A arr = {};
+  arr[2] = 1; // { dg-error "modifying a const object" }
+  return false;
+}
+
+constexpr bool b = f(); // { dg-message "in .constexpr. expansion of " }
+// { dg-message "originally declared" "" { target *-*-* } .-1 }
diff --git gcc/testsuite/g++.dg/cpp1z/constexpr-tracking-const1.C gcc/testsuite/g++.dg/cpp1z/constexpr-tracking-const1.C
new file mode 100644
index 00000000000..a3856b8e7ec
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1z/constexpr-tracking-const1.C
@@ -0,0 +1,25 @@ 
+// PR c++/91548 - fix detecting modifying const objects for ARRAY_REF.
+// { dg-do compile { target c++17 } }
+
+using size_t = decltype(sizeof(0));
+
+template <typename T, size_t N>
+constexpr T& impl(T const (&array)[N], size_t index) {
+    return const_cast<T&>(array[index]);
+}
+
+template <typename T, size_t N>
+struct my_array {
+    constexpr T& operator[](size_t i) { return impl(elems, i); }
+    constexpr T const& operator[](size_t i) const { return elems[i]; }
+    T elems[N];
+};
+
+bool f(int i) {
+    static constexpr auto table = []() {
+        my_array<bool, 256> arr = {};
+        arr[2] = true;
+        return arr;
+    }();
+    return table[i];
+}