On Tue, Sep 17, 2024 at 12:53:25PM -0400, Jason Merrill wrote:
> 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.
Oop, you're right, that's no good.
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.
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): If either
field's DECL_FIELD_OFFSET is null, compare the fields with ==.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Wtautological-compare4.C: New test.
---
gcc/fold-const.cc | 12 ++++++++---
.../g++.dg/warn/Wtautological-compare4.C | 21 +++++++++++++++++++
2 files changed, 30 insertions(+), 3 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..0578f42ac0c 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -3601,9 +3601,15 @@ 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)
+ || TREE_CODE (field1) != FIELD_DECL)
+ return false;
+
+ if (!DECL_FIELD_OFFSET (field0)
+ || !DECL_FIELD_OFFSET (field1))
+ return field0 == field1;
+
+ if (!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))
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: dfe0d4389a3ce43179563a63046ad3e74d615a08
--
2.46.0
