On 3/9/20 9:40 AM, Marek Polacek wrote:
On Mon, Mar 09, 2020 at 09:19:30AM -0400, Jason Merrill wrote:
On 3/9/20 8:58 AM, Jakub Jelinek wrote:
On Fri, Mar 06, 2020 at 07:43:43PM -0500, Jason Merrill wrote:
On 3/6/20 6:54 PM, Marek Polacek wrote:
I got a report that building Chromium fails with the "modifying a const
object" error.  After some poking I realized it's a bug in GCC, not in
their codebase.

Much like with ARRAY_REFs, which can be const even though the array
itself isn't, COMPONENT_REFs can be const although neither the object
nor the field were declared const.  So let's dial down the checking.
Here the COMPONENT_REF was const because of the "const_cast<const U &>(m)"
thing -- cxx_eval_component_reference then builds a COMPONENT_REF with
TREE_TYPE (t).

What is folding the const into the COMPONENT_REF?

cxx_eval_component_reference when it is called on
((const struct array *) this)->elems
with /*lval=*/true and lval is true because we are evaluating
<retval> = (const int &) &((const struct array *) 
this)->elems[VIEW_CONVERT_EXPR<size_t>(n)];

Ah, sure.  We're pretty loose with cv-quals in the constexpr code in
general, so it's probably not worth trying to change that here.  Getting
back to the patch:

Yes, here the additional const was caused by a const_cast adding a const.

But this could also happen with wrapper functions like this one from
__array_traits in std::array:

       static constexpr _Tp&
       _S_ref(const _Type& __t, std::size_t __n) noexcept
       { return const_cast<_Tp&>(__t[__n]); }

where the ref-to-const parameter added the const.

+      if (TREE_CODE (obj) == COMPONENT_REF)
+       {
+         tree op1 = TREE_OPERAND (obj, 1);
+         if (CP_TYPE_CONST_P (TREE_TYPE (op1)))
+           return true;
+         else
+           {
+             tree op0 = TREE_OPERAND (obj, 0);
+             /* The LHS of . or -> might itself be a COMPONENT_REF.  */
+             if (TREE_CODE (op0) == COMPONENT_REF)
+               op0 = TREE_OPERAND (op0, 1);
+             return CP_TYPE_CONST_P (TREE_TYPE (op0));
+           }
+       }

Shouldn't this be a loop?

I don't think so, though my earlier patch had a call to

+static bool
+cref_has_const_field (tree ref)
+{
+  while (TREE_CODE (ref) == COMPONENT_REF)
+    {
+      if (CP_TYPE_CONST_P (TREE_TYPE (TREE_OPERAND (ref, 1))))
+       return true;
+      ref = TREE_OPERAND (ref, 0);
+    }
+  return false;
+}

here.  A problem arised when I checked even the outermost expression (which is 
not a
field_decl), then I saw another problematical error.

The more outer fields are expected to be checked in subsequent calls to
modifying_const_object_p in next iterations of the

4459   for (tree probe = target; object == NULL_TREE; )

loop in cxx_eval_store_expression.

OK, but then why do you want to check two levels here rather than just one?

Jason

Reply via email to