On 1/21/26 8:10 PM, Jason Merrill wrote:
On 1/18/26 6:04 PM, Johannes Altmanninger wrote:
On this expression:

    (true ? "a" : "b")[index()]

"g++ -Wunused-value" incorrectly produces

    warning: left operand of comma operator has no effect [-Wunused- value]

 From the -fdump-tree-original output:

    if ((void) SAVE_EXPR <index ()>, 1)
      {
        (void) "a"[SAVE_EXPR <index ()>];
      }
    else
      {
        (void) "b"[SAVE_EXPR <index ()>];
      }

Observe that we evaluate index() (and save it) before evaluating the
ternary expression.  Since "(void) SAVE_EXPR <index ()>" is ostensibly
side-effect free, we get this warning.  Since SAVE_EXPR is not useless,
this is a false positive. Also the comma operator compiler-generated,
so warning about it is wrong.

Suppress this warning for this implicit expression. Test that the
warning is gone for "$ternary[index()]" but we still warn on cases like
"$ternary[(1, 0)]".

Thanks for the patch!

I'm pushing this adjusted version:

Alternatively, we could prevent all such warnings by changing the
definition of tem to "tree tem = cp_build_compound_expr (op0, idx,
tf_none)". That's probably the better solution but we'd need to check
if that would introduce false negatives.

Alternatively, we could set "side_effects_flag" for SAVE_EXPR, which
would also prevent this warning, but that feels wrong.

Other places in the compiler use warning_sentinel to temporarily disable a particular warning; instead of suppress_warning we could here use

               warning_sentinel w(warn_unused_value);

gcc/cp/ChangeLog:

    * typeck.cc (cp_build_array_ref): suppress unused-value

We capitalize the beginning of ChangeLog sentences (i.e. "Suppress").

    warning for implicit comma expression.

gcc/testsuite/ChangeLog:

    * g++.dg/cpp0x/Wunused-value2.C: New test.

I'd expect this test to go in warn/ rather than cpp0x/.


Signed-off-by: Johannes Altmanninger <[email protected]>
---
  gcc/cp/typeck.cc                            |  1 +
  gcc/testsuite/g++.dg/cpp0x/Wunused-value2.C | 18 ++++++++++++++++++
  2 files changed, 19 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wunused-value2.C

This is my first time hacking gcc, guidance is appreciated; I haven't
run the full test suite yet (which probably takes a long time when
built in debug mode, using
../gcc/configure --with-build-config=bootstrap-debug-lean --disable- bootstrap CFLAGS='-g -O0' CXXFLAGS='-g -O0').

It does take a while, but is still expected before submitting a patch.
Though I suppose this patch is small enough that just running Wunused* should be sufficient.

Instead, I've been using this command validate my change:

    make -j8 && timeout 5 make check-c++ RUNTESTFLAGS=dg.exp=g++.dg/ cpp0x/Wunused-value2.C | grep ^FAIL

without the "timeout" the command would go on to run other tests, like the "libgomp" ones etc.

You could make check-gcc-c++ instead to avoid that.

Probably I could use "runtest" directly.

I prefer to use make for parallel testing, but runtest is fine for a single test.

I'm pushing this adjusted version:

From 4ab3b3ce8d9b541c9dfe5d0820ebadad558046db Mon Sep 17 00:00:00 2001
From: Johannes Altmanninger <[email protected]>
Date: Mon, 26 Jan 2026 18:16:53 +0800
Subject: [PATCH] c++: -Wunused-value on ternary indexed by non-constant
To: [email protected]

On this expression:

	(true ? "a" : "b")[index()]

"g++ -Wunused-value" incorrectly produces

	warning: left operand of comma operator has no effect [-Wunused-value]

From the -fdump-tree-original output:

	if ((void) SAVE_EXPR <index ()>, 1)
	  {
	    (void) "a"[SAVE_EXPR <index ()>];
	  }
	else
	  {
	    (void) "b"[SAVE_EXPR <index ()>];
	  }

Observe that we evaluate index() (and save it) before evaluating the
ternary expression.  Since "(void) SAVE_EXPR <index ()>" is ostensibly
side-effect free, we get this warning.  Since SAVE_EXPR is not useless,
this is a false positive. Also the comma operator compiler-generated,
so warning about it is wrong.

Suppress this warning for this implicit expression. Test that the
warning is gone for "$ternary[index()]" but we still warn on cases like
"$ternary[(1, 0)]".

gcc/cp/ChangeLog:

	* typeck.cc (cp_build_array_ref): Suppress unused-value
	warning for implicit comma expression.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/Wunused-value-2.C: New test.

Signed-off-by: Johannes Altmanninger <[email protected]>
Co-authored-by: Jason Merrill <[email protected]>
---
 gcc/cp/typeck.cc                            |  1 +
 gcc/testsuite/g++.dg/warn/Wunused-value-2.C | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-value-2.C

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 980f2d115a6..40bb9828bcc 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -4244,6 +4244,7 @@ cp_build_array_ref (location_t loc, tree array, tree idx,
 	    {
 	      idx = save_expr (idx);
 	      op0 = save_expr (op0);
+	      warning_sentinel w (warn_unused_value);
 	      tree tem = build_compound_expr (loc, op0, idx);
 	      op0 = build_compound_expr (loc, tem, op0);
 	    }
diff --git a/gcc/testsuite/g++.dg/warn/Wunused-value-2.C b/gcc/testsuite/g++.dg/warn/Wunused-value-2.C
new file mode 100644
index 00000000000..8125aa518f0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wunused-value-2.C
@@ -0,0 +1,18 @@
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+static int index() { return 0; }
+
+volatile int global;
+static int index_with_side_effect() {
+    global += 1;
+    return 0;
+}
+
+int main(int argc, char **argv)
+{
+	const bool cond = argc == 10;
+	(void)(cond ? "" : "")[index()];
+	(void)(cond ? "" : "")[index_with_side_effect()];
+	(void)(cond ? "" : "")[(1, 0)]; // { dg-warning "left operand of comma operator has no effect" }
+}
-- 
2.52.0

Reply via email to