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) <case INDIRECT_REF>: 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.)
> {
> /* 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 complain)
> if (warn_unused_value
> && implicit != ICV_CAST
> && (complain & tf_warning)
> - && !warning_suppressed_p (expr, OPT_Wunused_value)
> - && !is_reference)
> + && !warning_suppressed_p (expr, OPT_Wunused_value)
> + && !REFERENCE_REF_P (expr))
> warning_at (loc, OPT_Wunused_value, "value computed is not
> used");
> expr = TREE_OPERAND (expr, 0);
> if (TREE_CODE (expr) == CALL_EXPR
> diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C
> b/gcc/testsuite/g++.dg/expr/discarded1a.C
> index 1c4dff4553e..4754f32f716 100644
> --- a/gcc/testsuite/g++.dg/expr/discarded1a.C
> +++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
> @@ -12,6 +12,5 @@ extern volatile Holder<Incomplete, 2>* c;
> int main() {
> a; // { dg-message "required from here" }
> b; // { dg-message "required from here" }
> - // { dg-warning "implicit dereference will not access object" "" { target
> *-*-* } .-1 }
> *c; // { dg-message "required from here" }
> }
> diff --git a/gcc/testsuite/g++.dg/expr/volatile2.C
> b/gcc/testsuite/g++.dg/expr/volatile2.C
> new file mode 100644
> index 00000000000..62e609aee2f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/expr/volatile2.C
> @@ -0,0 +1,12 @@
> +// Verify we preserve implicit loads of volatile references in a
> +// discarded-value expression.
> +// { dg-additional-options "-O -fdump-tree-original -fdump-tree-optimized" }
> +// { dg-final { scan-tree-dump {\*a} "original" } }
> +// { dg-final { scan-tree-dump {\*a} "optimized" } }
> +
> +struct A { int m; };
> +extern volatile A& a;
> +
> +int main() {
> + a;
> +}
> diff --git a/gcc/testsuite/g++.old-deja/g++.bugs/900428_01.C
> b/gcc/testsuite/g++.old-deja/g++.bugs/900428_01.C
> index a806ef0706e..2b82c64ff73 100644
> --- a/gcc/testsuite/g++.old-deja/g++.bugs/900428_01.C
> +++ b/gcc/testsuite/g++.old-deja/g++.bugs/900428_01.C
> @@ -46,16 +46,16 @@ void int_test (int i, int *p, volatile int *vp, int &r,
> volatile int &vr)
> (void)(i ? r : j); // ok, no warning
> (void)((void)1, r); // ok, no warning
>
> - vr; // { dg-warning "" } reference not
> accessed
> - (void)vr; // { dg-warning "" } reference not accessed
> - (void)(i ? vj : vr); // { dg-warning "" } reference not
> accessed
> - (void)(i ? vr : vj); // { dg-warning "" } reference not
> accessed
> - (void)((void)1, vr); // { dg-warning "" } reference not accessed
> + vr; // ok, no warning
> + (void)vr; // ok, no warning
> + (void)(i ? vj : vr); // ok, no warning
> + (void)(i ? vr : vj); // ok, no warning
> + (void)((void)1, vr); // ok, no warning
>
> *ip_fn (); // ok, no warning
> *vip_fn (); // ok, no warning
> ir_fn (); // ok, no warning
> - vir_fn (); // { dg-warning "" } reference not accessed
> + vir_fn (); // ok, no warning
> }
>
> struct S;
> @@ -128,16 +128,16 @@ void complete_test (int i, T *p, volatile T *vp, T &r,
> volatile T &vr)
> (void)(i ? r : j); // ok, no warning
> (void)((void)1, r); // ok, no warning
>
> - vr; // { dg-warning "" } reference not
> accessed
> - (void)vr; // { dg-warning "" } reference not accessed
> - (void)(i ? vj : vr); // { dg-warning "" } reference not
> accessed
> - (void)(i ? vr : vj); // { dg-warning "" } reference not
> accessed
> - (void)((void)1, vr); // { dg-warning "" } reference not accessed
> + vr; // ok, no warning
> + (void)vr; // ok, no warning
> + (void)(i ? vj : vr); // ok, no warning
> + (void)(i ? vr : vj); // ok, no warning
> + (void)((void)1, vr); // ok, no warning
>
> *tp_fn (); // ok, no warning
> *vtp_fn (); // ok, no warning
> tr_fn (); // ok, no warning
> - vtr_fn (); // ok, no warning{ dg-warning "" } reference
> not accessed
> + vtr_fn (); // ok, no warning
> }
>
> void extern_test ()
> @@ -160,5 +160,5 @@ void extern_test ()
> esr; // ok, no warning
> vesr; // { dg-warning "" } incomplete not
> accessed
> etr; // ok, no warning
> - vetr; // { dg-warning "" } reference not
> accessed
> + vetr; // ok, no warning
> }
> --
> 2.42.0.216.gbda494f404
>
>
