Re: [PATCH v2 2/2] c++: convert_to_void and volatile references
On 9/19/23 13:53, Patrick Palka wrote:
On Mon, 18 Sep 2023, Jason Merrill wrote:
On 9/18/23 12:12, Patrick Palka wrote:
Jason pointed out that even implicit loads of volatile references need
to undergo lvalue-to-rvalue conversion, but we currently emit a warning
in this case and discard the load. This patch changes this behavior so
that we don't issue a warning, and preserve the load.
gcc/cp/ChangeLog:
* cvt.cc (convert_to_void) : Remove warning
for an implicit load of a volatile reference. Simplify as if
is_reference is false. Check REFERENCE_REF_P in the test
guarding the -Wunused-value diagnostic.
gcc/testsuite/ChangeLog:
* g++.dg/expr/discarded1a.C: No longer expect warning for
implicit load of a volatile reference.
* g++.old-deja/g++.bugs/900428_01.C: Likewise.
* g++.dg/expr/volatile2.C: New test.
---
gcc/cp/cvt.cc | 56 ++-
gcc/testsuite/g++.dg/expr/discarded1a.C | 1 -
gcc/testsuite/g++.dg/expr/volatile2.C | 12
.../g++.old-deja/g++.bugs/900428_01.C | 26 -
4 files changed, 30 insertions(+), 65 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/expr/volatile2.C
diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
index 4424670356c..1cb6c1222c2 100644
--- a/gcc/cp/cvt.cc
+++ b/gcc/cp/cvt.cc
@@ -1251,12 +1251,9 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
{
tree type = TREE_TYPE (expr);
int is_volatile = TYPE_VOLATILE (type);
- if (is_volatile)
- complete_type (type);
- int is_complete = COMPLETE_TYPE_P (type);
/* Can't load the value if we don't know the type. */
- if (is_volatile && !is_complete)
+ if (is_volatile && !COMPLETE_TYPE_P (complete_type (type)))
{
if (complain & tf_warning)
switch (implicit)
@@ -1298,50 +1295,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
gcc_unreachable ();
}
}
- /* Don't load the value if this is an implicit dereference, or if
- the type needs to be handled by ctors/dtors. */
- else if (is_volatile && is_reference)
- {
-if (complain & tf_warning)
- switch (implicit)
- {
- case ICV_CAST:
- warning_at (loc, 0, "conversion to void will not access "
- "object of type %qT", type);
- break;
- case ICV_SECOND_OF_COND:
- warning_at (loc, 0, "implicit dereference will not access
"
- "object of type %qT in second operand of "
- "conditional expression", type);
- break;
- case ICV_THIRD_OF_COND:
- warning_at (loc, 0, "implicit dereference will not access
"
- "object of type %qT in third operand of "
- "conditional expression", type);
- break;
- case ICV_RIGHT_OF_COMMA:
- warning_at (loc, 0, "implicit dereference will not access
"
- "object of type %qT in right operand of "
- "comma operator", type);
- break;
- case ICV_LEFT_OF_COMMA:
- warning_at (loc, 0, "implicit dereference will not access
"
- "object of type %qT in left operand of comma "
- "operator", type);
- break;
- case ICV_STATEMENT:
- warning_at (loc, 0, "implicit dereference will not access
"
- "object of type %qT in statement", type);
-break;
- case ICV_THIRD_IN_FOR:
- warning_at (loc, 0, "implicit dereference will not access
"
- "object of type %qT in for increment
expression",
- type);
- break;
- default:
- gcc_unreachable ();
- }
- }
+ /* Don't load the value if the type needs to be handled by cdtors. */
else if (is_volatile && TREE_ADDRESSABLE (type))
{
if (complain & tf_warning)
@@ -1386,7 +1340,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
gcc_unreachable ();
}
}
- if (is_reference || !is_volatile || !is_complete || TREE_ADDRESSABLE
(type))
+ if (!is_volatile || !COMPLETE_TYPE_P (type))
{
/* Emit a warning (if enabled) when the "effect-less"
INDIRECT_REF
operation is stripped off. Note that we don't warn about
Re: [PATCH v2 2/2] c++: convert_to_void and volatile references
On Mon, 18 Sep 2023, Jason Merrill wrote:
> On 9/18/23 12:12, Patrick Palka wrote:
> > Jason pointed out that even implicit loads of volatile references need
> > to undergo lvalue-to-rvalue conversion, but we currently emit a warning
> > in this case and discard the load. This patch changes this behavior so
> > that we don't issue a warning, and preserve the load.
> >
> > gcc/cp/ChangeLog:
> >
> > * cvt.cc (convert_to_void) : Remove warning
> > for an implicit load of a volatile reference. Simplify as if
> > is_reference is false. Check REFERENCE_REF_P in the test
> > guarding the -Wunused-value diagnostic.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/expr/discarded1a.C: No longer expect warning for
> > implicit load of a volatile reference.
> > * g++.old-deja/g++.bugs/900428_01.C: Likewise.
> > * g++.dg/expr/volatile2.C: New test.
> > ---
> > gcc/cp/cvt.cc | 56 ++-
> > gcc/testsuite/g++.dg/expr/discarded1a.C | 1 -
> > gcc/testsuite/g++.dg/expr/volatile2.C | 12
> > .../g++.old-deja/g++.bugs/900428_01.C | 26 -
> > 4 files changed, 30 insertions(+), 65 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/expr/volatile2.C
> >
> > diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
> > index 4424670356c..1cb6c1222c2 100644
> > --- a/gcc/cp/cvt.cc
> > +++ b/gcc/cp/cvt.cc
> > @@ -1251,12 +1251,9 @@ convert_to_void (tree expr, impl_conv_void implicit,
> > tsubst_flags_t complain)
> > {
> > tree type = TREE_TYPE (expr);
> > int is_volatile = TYPE_VOLATILE (type);
> > - if (is_volatile)
> > - complete_type (type);
> > - int is_complete = COMPLETE_TYPE_P (type);
> > /* Can't load the value if we don't know the type. */
> > - if (is_volatile && !is_complete)
> > + if (is_volatile && !COMPLETE_TYPE_P (complete_type (type)))
> > {
> > if (complain & tf_warning)
> > switch (implicit)
> > @@ -1298,50 +1295,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
> > tsubst_flags_t complain)
> > gcc_unreachable ();
> > }
> > }
> > - /* Don't load the value if this is an implicit dereference, or if
> > - the type needs to be handled by ctors/dtors. */
> > - else if (is_volatile && is_reference)
> > - {
> > -if (complain & tf_warning)
> > - switch (implicit)
> > - {
> > - case ICV_CAST:
> > - warning_at (loc, 0, "conversion to void will not access "
> > - "object of type %qT", type);
> > - break;
> > - case ICV_SECOND_OF_COND:
> > - warning_at (loc, 0, "implicit dereference will not access
> > "
> > - "object of type %qT in second operand of "
> > - "conditional expression", type);
> > - break;
> > - case ICV_THIRD_OF_COND:
> > - warning_at (loc, 0, "implicit dereference will not access
> > "
> > - "object of type %qT in third operand of "
> > - "conditional expression", type);
> > - break;
> > - case ICV_RIGHT_OF_COMMA:
> > - warning_at (loc, 0, "implicit dereference will not access
> > "
> > - "object of type %qT in right operand of "
> > - "comma operator", type);
> > - break;
> > - case ICV_LEFT_OF_COMMA:
> > - warning_at (loc, 0, "implicit dereference will not access
> > "
> > - "object of type %qT in left operand of comma "
> > - "operator", type);
> > - break;
> > - case ICV_STATEMENT:
> > - warning_at (loc, 0, "implicit dereference will not access
> > "
> > - "object of type %qT in statement", type);
> > -break;
> > - case ICV_THIRD_IN_FOR:
> > - warning_at (loc, 0, "implicit dereference will not access
> > "
> > - "object of type %qT in for increment
> > expression",
> > - type);
> > - break;
> > - default:
> > - gcc_unreachable ();
> > - }
> > - }
> > + /* Don't load the value if the type needs to be handled by cdtors. */
> > else if (is_volatile && TREE_ADDRESSABLE (type))
> > {
> > if (complain & tf_warning)
> > @@ -1386,7 +1340,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
> > tsubst_flags_t complain)
> > gcc_unreachable ();
> > }
> > }
> > - if (is_reference || !is_volatile || !is_complete || TREE_ADDRESSABLE
> > (type))
> > + if (!is_volatile || !COMPLETE_TYPE_P (type))
> > {
> > /* Emit
Re: [PATCH v2 2/2] c++: convert_to_void and volatile references
On 9/18/23 12:12, Patrick Palka wrote:
Jason pointed out that even implicit loads of volatile references need
to undergo lvalue-to-rvalue conversion, but we currently emit a warning
in this case and discard the load. This patch changes this behavior so
that we don't issue a warning, and preserve the load.
gcc/cp/ChangeLog:
* cvt.cc (convert_to_void) : Remove warning
for an implicit load of a volatile reference. Simplify as if
is_reference is false. Check REFERENCE_REF_P in the test
guarding the -Wunused-value diagnostic.
gcc/testsuite/ChangeLog:
* g++.dg/expr/discarded1a.C: No longer expect warning for
implicit load of a volatile reference.
* g++.old-deja/g++.bugs/900428_01.C: Likewise.
* g++.dg/expr/volatile2.C: New test.
---
gcc/cp/cvt.cc | 56 ++-
gcc/testsuite/g++.dg/expr/discarded1a.C | 1 -
gcc/testsuite/g++.dg/expr/volatile2.C | 12
.../g++.old-deja/g++.bugs/900428_01.C | 26 -
4 files changed, 30 insertions(+), 65 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/expr/volatile2.C
diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
index 4424670356c..1cb6c1222c2 100644
--- a/gcc/cp/cvt.cc
+++ b/gcc/cp/cvt.cc
@@ -1251,12 +1251,9 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
{
tree type = TREE_TYPE (expr);
int is_volatile = TYPE_VOLATILE (type);
- if (is_volatile)
- complete_type (type);
- int is_complete = COMPLETE_TYPE_P (type);
/* Can't load the value if we don't know the type. */
- if (is_volatile && !is_complete)
+ if (is_volatile && !COMPLETE_TYPE_P (complete_type (type)))
{
if (complain & tf_warning)
switch (implicit)
@@ -1298,50 +1295,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
gcc_unreachable ();
}
}
- /* Don't load the value if this is an implicit dereference, or if
- the type needs to be handled by ctors/dtors. */
- else if (is_volatile && is_reference)
- {
-if (complain & tf_warning)
- switch (implicit)
- {
- case ICV_CAST:
- warning_at (loc, 0, "conversion to void will not access "
- "object of type %qT", type);
- break;
- case ICV_SECOND_OF_COND:
- warning_at (loc, 0, "implicit dereference will not access "
- "object of type %qT in second operand of "
- "conditional expression", type);
- break;
- case ICV_THIRD_OF_COND:
- warning_at (loc, 0, "implicit dereference will not access "
- "object of type %qT in third operand of "
- "conditional expression", type);
- break;
- case ICV_RIGHT_OF_COMMA:
- warning_at (loc, 0, "implicit dereference will not access "
- "object of type %qT in right operand of "
- "comma operator", type);
- break;
- case ICV_LEFT_OF_COMMA:
- warning_at (loc, 0, "implicit dereference will not access "
- "object of type %qT in left operand of comma "
- "operator", type);
- break;
- case ICV_STATEMENT:
- warning_at (loc, 0, "implicit dereference will not access "
- "object of type %qT in statement", type);
-break;
- case ICV_THIRD_IN_FOR:
- warning_at (loc, 0, "implicit dereference will not access "
- "object of type %qT in for increment
expression",
- type);
- break;
- default:
- gcc_unreachable ();
- }
- }
+ /* Don't load the value if the type needs to be handled by cdtors. */
else if (is_volatile && TREE_ADDRESSABLE (type))
{
if (complain & tf_warning)
@@ -1386,7 +1340,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
gcc_unreachable ();
}
}
- if (is_reference || !is_volatile || !is_complete || TREE_ADDRESSABLE
(type))
+ if (!is_volatile || !COMPLETE_TYPE_P (type))
{
/* Emit a warning (if enabled) when the "effect-less" INDIRECT_REF
operation is stripped off. Note that we don't warn about
@@ -1397,8 +1351,8 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t
Re: [PATCH v2 2/2] c++: convert_to_void and volatile references
On Mon, 18 Sep 2023, Patrick Palka wrote:
> Jason pointed out that even implicit loads of volatile references need
> to undergo lvalue-to-rvalue conversion, but we currently emit a warning
> in this case and discard the load. This patch changes this behavior so
> that we don't issue a warning, and preserve the load.
>
> gcc/cp/ChangeLog:
>
> * cvt.cc (convert_to_void) : Remove warning
> for an implicit load of a volatile reference. Simplify as if
> is_reference is false. Check REFERENCE_REF_P in the test
> guarding the -Wunused-value diagnostic.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/expr/discarded1a.C: No longer expect warning for
> implicit load of a volatile reference.
> * g++.old-deja/g++.bugs/900428_01.C: Likewise.
> * g++.dg/expr/volatile2.C: New test.
> ---
> gcc/cp/cvt.cc | 56 ++-
> gcc/testsuite/g++.dg/expr/discarded1a.C | 1 -
> gcc/testsuite/g++.dg/expr/volatile2.C | 12
> .../g++.old-deja/g++.bugs/900428_01.C | 26 -
> 4 files changed, 30 insertions(+), 65 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/expr/volatile2.C
>
> diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
> index 4424670356c..1cb6c1222c2 100644
> --- a/gcc/cp/cvt.cc
> +++ b/gcc/cp/cvt.cc
> @@ -1251,12 +1251,9 @@ convert_to_void (tree expr, impl_conv_void implicit,
> tsubst_flags_t complain)
>{
> tree type = TREE_TYPE (expr);
> int is_volatile = TYPE_VOLATILE (type);
> - if (is_volatile)
> - complete_type (type);
> - int is_complete = COMPLETE_TYPE_P (type);
>
> /* Can't load the value if we don't know the type. */
> - if (is_volatile && !is_complete)
> + if (is_volatile && !COMPLETE_TYPE_P (complete_type (type)))
>{
> if (complain & tf_warning)
> switch (implicit)
> @@ -1298,50 +1295,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
> tsubst_flags_t complain)
> gcc_unreachable ();
> }
>}
> - /* Don't load the value if this is an implicit dereference, or if
> -the type needs to be handled by ctors/dtors. */
> - else if (is_volatile && is_reference)
> - {
> -if (complain & tf_warning)
> - switch (implicit)
> - {
> - case ICV_CAST:
> - warning_at (loc, 0, "conversion to void will not access "
> - "object of type %qT", type);
> - break;
> - case ICV_SECOND_OF_COND:
> - warning_at (loc, 0, "implicit dereference will not access "
> - "object of type %qT in second operand of "
> - "conditional expression", type);
> - break;
> - case ICV_THIRD_OF_COND:
> - warning_at (loc, 0, "implicit dereference will not access "
> - "object of type %qT in third operand of "
> - "conditional expression", type);
> - break;
> - case ICV_RIGHT_OF_COMMA:
> - warning_at (loc, 0, "implicit dereference will not access "
> - "object of type %qT in right operand of "
> - "comma operator", type);
> - break;
> - case ICV_LEFT_OF_COMMA:
> - warning_at (loc, 0, "implicit dereference will not access "
> - "object of type %qT in left operand of comma "
> - "operator", type);
> - break;
> - case ICV_STATEMENT:
> - warning_at (loc, 0, "implicit dereference will not access "
> - "object of type %qT in statement", type);
> - break;
> - case ICV_THIRD_IN_FOR:
> - warning_at (loc, 0, "implicit dereference will not access "
> - "object of type %qT in for increment
> expression",
> - type);
> - break;
> - default:
> - gcc_unreachable ();
> - }
> - }
> + /* Don't load the value if the type needs to be handled by cdtors. */
> else if (is_volatile && TREE_ADDRESSABLE (type))
> {
> if (complain & tf_warning)
> @@ -1386,7 +1340,7 @@ convert_to_void (tree expr, impl_conv_void implicit,
> tsubst_flags_t complain)
> gcc_unreachable ();
> }
> }
> - if (is_reference || !is_volatile || !is_complete || TREE_ADDRESSABLE
> (type))
> + if (!is_volatile || !COMPLETE_TYPE_P (type))
... and the TREE_ADDRESSABLE test should not have been removed, consider
that fixed. (The original patch doesn't have this mistake, I just
messed up splitting the patch into two.)
>
