On 9/16/24 7:08 PM, Marek Polacek wrote:
On Tue, Sep 10, 2024 at 10:37:31AM -0400, Jason Merrill wrote:
On 8/29/24 12:23 PM, Marek Polacek wrote:
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/14?

-- >8 --
Pre r14-4793, we'd call warn_tautological_cmp -> operand_equal_p
with operands wrapped in NON_DEPENDENT_EXPR, which works, since
o_e_p bails for codes it doesn't know.  But now we pass operands
not encapsulated in NON_DEPENDENT_EXPR, and crash, because the
template tree for &a[x] has null DECL_FIELD_OFFSET.

Why are we trying to compare DECL_FIELD_OFFSET in C++, anyway?  I'd think we
should limit that to C and in C++ rely on FIELD_DECL identity.

This goes back to r12-7797.  I suppose it's trying to handle C++ because we
can't use c_dialect_cxx () in fold-const.  I know there's lang_GNU_CXX but
I suggest we do the following.
I don't think I meant this warning to be called in a template,
so let's avoid the problem this easy way.

Hmm, it seems potentially useful in a template, especially since we suppress
it in instantiations.

Ack.

How about this?

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Pre r14-4793, we'd call warn_tautological_cmp -> operand_equal_p
with operands wrapped in NON_DEPENDENT_EXPR, which works, since
o_e_p bails for codes it doesn't know.  But now we pass operands
not encapsulated in NON_DEPENDENT_EXPR, and crash, because the
template tree for &a[x] has null DECL_FIELD_OFFSET.

This patch extends r12-7797 to cover the case when DECL_FIELD_OFFSET
is null.

        PR c++/116534

gcc/ChangeLog:

        * fold-const.cc (operand_compare::operand_equal_p): Check
        DECL_FIELD_OFFSET.

gcc/testsuite/ChangeLog:

        * g++.dg/warn/Wtautological-compare4.C: New test.
---
  gcc/fold-const.cc                             | 16 ++++++++------
  .../g++.dg/warn/Wtautological-compare4.C      | 21 +++++++++++++++++++
  2 files changed, 31 insertions(+), 6 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Wtautological-compare4.C

diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 70db16759d0..3da09621584 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -3601,12 +3601,16 @@ operand_compare::operand_equal_p (const_tree arg0, 
const_tree arg1,
/* Non-FIELD_DECL operands can appear in C++ templates. */
                    if (TREE_CODE (field0) != FIELD_DECL
-                       || TREE_CODE (field1) != FIELD_DECL
-                       || !operand_equal_p (DECL_FIELD_OFFSET (field0),
-                                            DECL_FIELD_OFFSET (field1), flags)
-                       || !operand_equal_p (DECL_FIELD_BIT_OFFSET (field0),
-                                            DECL_FIELD_BIT_OFFSET (field1),
-                                            flags))
+                       || TREE_CODE (field1) != FIELD_DECL)
+                     return false;
+
+                   if (DECL_FIELD_OFFSET (field0)
+                       && DECL_FIELD_OFFSET (field1)
+                       && (!operand_equal_p (DECL_FIELD_OFFSET (field0),
+                                             DECL_FIELD_OFFSET (field1), flags)
+                           || !operand_equal_p (DECL_FIELD_BIT_OFFSET (field0),
+                                                DECL_FIELD_BIT_OFFSET (field1),
+                                                flags)))

If at least one has null FIELD_OFFSET, I'd think we still want to compare them for being ==, this seems to assume that a field with null offset is equal to any other field.

                      return false;
                  }
                else
diff --git a/gcc/testsuite/g++.dg/warn/Wtautological-compare4.C 
b/gcc/testsuite/g++.dg/warn/Wtautological-compare4.C
new file mode 100644
index 00000000000..96308f49a42
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wtautological-compare4.C
@@ -0,0 +1,21 @@
+// PR c++/116534
+// { dg-do compile }
+// { dg-options "-Wall" }
+
+template <class A>
+struct Test {
+    bool foo(unsigned x, unsigned y) {
+        bool test = &a[x] == &b[y];
+       return test;
+    }
+    unsigned *a;
+    unsigned *b;
+};
+
+void
+g ()
+{
+  Test<int> t;
+  t.foo (0u, 1u);
+  t.foo (0u, 0u);
+}

base-commit: f5448384a2134f32c8733b401440da11bfe69252

Reply via email to