Re: [PATCH] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Jason Merrill via Gcc-patches]

On 3/9/20 4:34 PM, Marek Polacek wrote:

On Mon, Mar 09, 2020 at 04:25:00PM -0400, Marek Polacek wrote:

On Mon, Mar 09, 2020 at 03:37:56PM -0400, Jason Merrill wrote:

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(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
 = (const int &) &((const struct array *) 
this)->elems[VIEW_CONVERT_EXPR(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?


It's a hack to keep constexpr-tracking-const7.C working.  There we have

   b.a.c.d.n

wherein 'd' is const struct D, but 'n' isn't const.  Without the hack
const_object_being_modified would be 'b.a.c.d', but due to the problem I
desribed in the original mail[1] the constructor for D wouldn't have
TREE_READONLY set.  With the hack const_object_being_modified will be
'b.a.c.d.n', which is of non-class type so we error:

4710   if (!CLASS_TYPE_P (const_objtype))
4711 fail = true;

I could remove the hack and maybe XFAIL constexpr-tracking-const7.C if you
want.  Unfortunately I wasn't aware of [1] when I added that feature and
checking if the whole COMPONENT_REF is const seemed to be enough.


So if D was a wrapper around another class with the int field, this hack 
looking one level out wouldn't help?



It's probably not a good idea to make this checking more strict at this
stage.

[1] "While looking into this I noticed that we don't detect modifying a const
object in certain cases like in
.  That's because
we never evaluate an X::X() CALL_EXPR -- there's none.  So there's no
CONSTRUCTOR to set TREE_READONLY on.  No idea how to fix this, but it's
likely something for GCC 11 anyway."

How about this?

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 76af0d710c4..b3d3499b9ac 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4759,6 +4759,14 @@ cxx_eval_store_expression (const constexpr_ctx *ctx, tree t,
   else
 *valp = init;
 
+  /* After initialization, 'const' semantics apply to the value of the
+ object. Make a note of this fact by marking the CONSTRUCTOR
+ TREE_READONLY.  */
+  if (TREE_CODE (t) == INIT_EXPR
+  && TREE_CODE (*valp) == CONSTRUCTOR
+  && TYPE_READONLY (type))
+TREE_READONLY (*valp) = true;
+
   /* Update TREE_CONSTANT and TREE_SIDE_EFFECTS on enclosing
  CONSTRUCTORs, if any.  */
   tree elt;

Re: [PATCH] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Marek Polacek]

On Mon, Mar 09, 2020 at 04:25:00PM -0400, Marek Polacek wrote:
> On Mon, Mar 09, 2020 at 03:37:56PM -0400, Jason Merrill wrote:
> > 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 > > > > > > 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
> > > > >  = (const int &) &((const struct array *) 
> > > > > this)->elems[VIEW_CONVERT_EXPR(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?
> 
> It's a hack to keep constexpr-tracking-const7.C working.  There we have
> 
>   b.a.c.d.n
> 
> wherein 'd' is const struct D, but 'n' isn't const.  Without the hack
> const_object_being_modified would be 'b.a.c.d', but due to the problem I
> desribed in the original mail[1] the constructor for D wouldn't have
> TREE_READONLY set.  With the hack const_object_being_modified will be
> 'b.a.c.d.n', which is of non-class type so we error:
> 
> 4710   if (!CLASS_TYPE_P (const_objtype))
> 4711 fail = true;
> 
> I could remove the hack and maybe XFAIL constexpr-tracking-const7.C if you
> want.  Unfortunately I wasn't aware of [1] when I added that feature and
> checking if the whole COMPONENT_REF is const seemed to be enough.
> 
> It's probably not a good idea to make this checking more strict at this
> stage.
> 
> [1] "While looking into this I noticed that we don't detect modifying a const
> object in certain cases like in
> .  That's because
> we never evaluate an X::X() CALL_EXPR -- there's none.  So there's no
> CONSTRUCTOR to set TREE_READONLY on.  No idea how to fix this, but it's
> likely something for GCC 11 anyway."

The testcase disappeared from Bugzilla, but it was

Re: [PATCH] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Marek Polacek]

On Mon, Mar 09, 2020 at 03:37:56PM -0400, Jason Merrill wrote:
> 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 > > > > > &>(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
> > > >  = (const int &) &((const struct array *) 
> > > > this)->elems[VIEW_CONVERT_EXPR(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?

It's a hack to keep constexpr-tracking-const7.C working.  There we have

  b.a.c.d.n

wherein 'd' is const struct D, but 'n' isn't const.  Without the hack
const_object_being_modified would be 'b.a.c.d', but due to the problem I
desribed in the original mail[1] the constructor for D wouldn't have
TREE_READONLY set.  With the hack const_object_being_modified will be
'b.a.c.d.n', which is of non-class type so we error:

4710   if (!CLASS_TYPE_P (const_objtype))
4711 fail = true;

I could remove the hack and maybe XFAIL constexpr-tracking-const7.C if you
want.  Unfortunately I wasn't aware of [1] when I added that feature and
checking if the whole COMPONENT_REF is const seemed to be enough.

It's probably not a good idea to make this checking more strict at this
stage.

[1] "While looking into this I noticed that we don't detect modifying a const
object in certain cases like in
.  That's because
we never evaluate an X::X() CALL_EXPR -- there's none.  So there's no
CONSTRUCTOR to set TREE_READONLY on.  No idea how to fix this, but it's
likely something for GCC 11 anyway."

Marek

Re: [PATCH] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Jason Merrill]

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(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
 = (const int &) &((const struct array *) 
this)->elems[VIEW_CONVERT_EXPR(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

Re: [PATCH] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Marek Polacek]

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 > > > &>(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
> >  = (const int &) &((const struct array *) 
> > this)->elems[VIEW_CONVERT_EXPR(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.

Marek

Re: [PATCH] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Jason Merrill]

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(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
 = (const int &) &((const struct array *) 
this)->elems[VIEW_CONVERT_EXPR(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:



+  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?

Jason

Re: [PATCH] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Jakub Jelinek]

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(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
 = (const int &) &((const struct array *) 
this)->elems[VIEW_CONVERT_EXPR(n)];

Jakub

Re: [PATCH] c++: Fix wrong modifying const object error for COMPONENT_REF [PR94074]

[Jason Merrill]

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(m)"
thing -- cxx_eval_component_reference then builds a COMPONENT_REF with
TREE_TYPE (t).


What is folding the const into the COMPONENT_REF?  Should that build a 
VIEW_CONVERT_EXPR instead?



While looking into this I noticed that we don't detect modifying a const
object in certain cases like in
.  That's because
we never evaluate an X::X() CALL_EXPR -- there's none.  So there's no
CONSTRUCTOR to set TREE_READONLY on.  No idea how to fix this, but it's
likely something for GCC 11 anyway.

PR c++/94074 - wrong modifying const object error for COMPONENT_REF.
* constexpr.c (modifying_const_object_p): Consider a COMPONENT_REF
const only if its object or field are const.

* g++.dg/cpp1y/constexpr-tracking-const17.C: New test.
* g++.dg/cpp1y/constexpr-tracking-const18.C: New test.
* g++.dg/cpp1y/constexpr-tracking-const19.C: New test.
* g++.dg/cpp1y/constexpr-tracking-const20.C: New test.
---
  gcc/cp/constexpr.c| 30 ++-
  .../g++.dg/cpp1y/constexpr-tracking-const17.C | 23 ++
  .../g++.dg/cpp1y/constexpr-tracking-const18.C | 23 ++
  .../g++.dg/cpp1y/constexpr-tracking-const19.C | 23 ++
  .../g++.dg/cpp1y/constexpr-tracking-const20.C | 28 +
  5 files changed, 126 insertions(+), 1 deletion(-)
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const17.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const18.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const19.C
  create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const20.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 521c87f6210..936d171b9e4 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4401,7 +4401,35 @@ modifying_const_object_p (tree_code code, tree obj, bool 
mutable_p)
if (mutable_p)
  return false;
  
-  return (TREE_READONLY (obj) || CP_TYPE_CONST_P (TREE_TYPE (obj)));

+  if (TREE_READONLY (obj))
+return true;
+
+  if (CP_TYPE_CONST_P (TREE_TYPE (obj)))
+{
+  /* Although a COMPONENT_REF may have a const type, we should
+only consider it modifying a const object when the field
+or object components is const.  This can happen when using
+constructs such as const_cast(m), making something
+const even though it wasn't declared 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));
+   }
+   }
+  else
+   return true;
+}
+
+  return false;
  }
  
  /* Evaluate an INIT_EXPR or MODIFY_EXPR.  */

diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const17.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const17.C
new file mode 100644
index 000..3f215d28175
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const17.C
@@ -0,0 +1,23 @@
+// PR c++/94074 - wrong modifying const object error for COMPONENT_REF.
+// { dg-do compile { target c++14 } }
+
+typedef decltype (sizeof (0)) size_t;
+
+template 
+struct array
+{
+  constexpr const E [](size_t n) const noexcept { return elems[n]; }
+  E elems[N];
+};
+
+template 
+struct S {
+  using U = array;
+  U m;
+  constexpr S(int) : m{}
+  {
+const_cast(const_cast(m)[0]) = 42;
+  }
+};
+
+constexpr S p = { 10 };
diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const18.C 
b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const18.C
new file mode 100644
index 000..11a680468c2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-tracking-const18.C
@@ -0,0 +1,23 @@
+// PR c++/94074 - wrong modifying const object error for COMPONENT_REF.
+// { dg-do compile { target c++14 } }
+
+typedef decltype (sizeof (0)) size_t;
+
+template 
+struct array
+{
+  constexpr const E [](size_t n) const noexcept { return elems[n]; }
+  E elems[N];
+};
+
+template 
+struct S {
+  using U = array;
+  const U m;
+  constexpr S(int) : m{}
+  {
+const_cast(const_cast(m)[0]) = 42; // { dg-error "modifying a 
const object" }
+  }