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) <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))
             {
               /* 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

This should still warn (and not load), a function call is not one of the forms
in https://eel.is/c++draft/expr#context-2.  We still need the warnings you're
removing for expression forms that aren't on that list.

D'oh, will fix.  It's surprising at first glance that *vip_fn() behaves
differently from vir_fn().

Yeah, the rationale was that in *vip_fn the last explicit semantic is the dereference, while in vir_fn() it's the call. It is pretty subtle, though.

Jason

Reply via email to