[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 --- Comment #8 from CVS Commits --- The master branch has been updated by Jakub Jelinek : https://gcc.gnu.org/g:8698ff67cdff4364c8adad2921ed532359a155ec commit r12-7797-g8698ff67cdff4364c8adad2921ed532359a155ec Author: Jakub Jelinek Date: Thu Mar 24 12:23:51 2022 +0100 fold-const: Handle C++ dependent COMPONENT_REFs in operand_equal_p [PR105035] As mentioned in the PR, operand_equal_p already contains some hacks so that it can be called already on pre-instantiation C++ trees from templates, but the recent change to compare DECL_FIELD_OFFSET in the COMPONENT_REF case broke this. Many such COMPONENT_REFs are already punted on earlier because they have NULL TREE_TYPE, but in this case the code knows what type they have but still uses an IDENTIFIER_NODE as second operand of COMPONENT_REF (I think SCOPE_REF is something that could be used too). The following patch looks at those DECL_FIELD_*OFFSET fields only if both field[01] args are FIELD_DECLs and otherwise keeps it to the earlier OP_SAME (1) check that guards this whole block. 2022-03-24 Jakub Jelinek PR c++/105035 * fold-const.cc (operand_equal_p) : If either field0 or field1 is not a FIELD_DECL, return false. * g++.dg/warn/Wduplicated-cond2.C: New test.
[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035
--- Comment #7 from Patrick Palka ---
(In reply to Patrick Palka from comment #6)
> (In reply to Jakub Jelinek from comment #3)
> > Another option is to make sure we don't call
> > warn_duplicated_cond_add_or_warn
> > when processing_template_decl or say when value_dependent_expression_p or
> > similar, and instead call it during template instantiation in pt.cc after
> > finish_if_stmt_cond call there.
>
> I noticed the warning already gives up on conditions that have
> TREE_SIDE_EFFECTS set, so I suppose it makes sense to do the same for
> type-dependent expressions since we can't know if they have side effects
> until instantiation time.
However, doing the check at instantiation time for dependent conditions might
be problematic since two different dependent conditions could instantiate to
the same non-dependent condition, and I don't think we'd want to issue a
-Wduplicated-cond warning in this case. For example,
template
void f() {
if (T() == 5)
...
else if (U() == 5)
...
}
I'm not sure issuing a warning at instantiation time for f() would be
appropriate. We'd might have to compare the dependent conditions even at
instantiation time, provided that the instantiated condition doesn't have
TREE_SIDE_EFFECTS set... Seems quite messy to get right.
[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 Patrick Palka changed: What|Removed |Added CC||ppalka at gcc dot gnu.org --- Comment #6 from Patrick Palka --- (In reply to Jakub Jelinek from comment #3) > Another option is to make sure we don't call warn_duplicated_cond_add_or_warn > when processing_template_decl or say when value_dependent_expression_p or > similar, and instead call it during template instantiation in pt.cc after > finish_if_stmt_cond call there. I noticed the warning already gives up on conditions that have TREE_SIDE_EFFECTS set, so I suppose it makes sense to do the same for type-dependent expressions since we can't know if they have side effects until instantiation time. And I wonder if warn_duplicated_cond_add_or_warn should be using cp_tree_equal instead operand_equal_p? IIUC the latter doesn't handle C++-specific tree codes.
[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 Jakub Jelinek changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #5 from Jakub Jelinek --- Created attachment 52671 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52671=edit gcc12-pr105035.patch Untested fix.
[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 --- Comment #4 from rguenther at suse dot de --- On Wed, 23 Mar 2022, jakub at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 > > Jakub Jelinek changed: > >What|Removed |Added > > CC||jakub at gcc dot gnu.org, >||mpolacek at gcc dot gnu.org > > --- Comment #3 from Jakub Jelinek --- > I think because operand_equal_p already has code to deal with the C++ special > trees and because it is such a low level API, changing it seems to be better. Agreed.
[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 Jakub Jelinek changed: What|Removed |Added CC||jakub at gcc dot gnu.org, ||mpolacek at gcc dot gnu.org --- Comment #3 from Jakub Jelinek --- The C++ FE when parsing templates (processing_template_decl is true) produces a lot of trees the middle-end doesn't like. If they aren't type or value dependent, usually (but not always) the trees are such that they can be handed over to the generic code even when parsing templates. The -Wduplicated-cond warning implemented in PR64249 seems to do: condition = finish_if_stmt_cond (condition, statement); if (warn_duplicated_cond) warn_duplicated_cond_add_or_warn (token->location, condition, ); only in the parser regardless of processing_template_decl. So, the options are either to make sure at least operand_equal_p can cope even with those trees and apparently operand_equal_p already has such spots in it, e.g.: /* Similar, if either does not have a type (like a template id), they aren't equal. */ if (!TREE_TYPE (arg0) || !TREE_TYPE (arg1)) return false; So, in this particular case, we'd punt if either field0 or field1 is not a FIELD_DECL (this is in !OP_SAME (1) guarded code). Another option is to make sure we don't call warn_duplicated_cond_add_or_warn when processing_template_decl or say when value_dependent_expression_p or similar, and instead call it during template instantiation in pt.cc after finish_if_stmt_cond call there. I think because operand_equal_p already has code to deal with the C++ special trees and because it is such a low level API, changing it seems to be better.
[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 Richard Biener changed: What|Removed |Added Keywords||ice-on-valid-code Priority|P3 |P2
[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 Richard Biener changed: What|Removed |Added CC||jason at redhat dot com, ||rguenth at gcc dot gnu.org --- Comment #2 from Richard Biener --- It looks like the C++ frontend builds COMPONENT_REF with IDENTIFIER_NODE operand one and the middle-end is not expecting this. Maybe we call operand_equal_p before layout is complete? I suppose we could guard operand_equal_p for this case but then handing invalid GENERIC to the middle-end is suspicious. Jason?
[Bug c++/105035] [11/12 Regression] tree check: expected field_decl, have identifier_node in operand_equal_p, at fold-const.c:3335 since r11-5181-g0862d007b564eca8
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105035 Martin Liška changed: What|Removed |Added Last reconfirmed||2022-03-23 Ever confirmed|0 |1 Status|UNCONFIRMED |NEW Target Milestone|--- |11.3 Summary|regression ICE segmentation |[11/12 Regression] tree |fault with |check: expected field_decl, |-Wduplicated-cond |have identifier_node in ||operand_equal_p, at ||fold-const.c:3335 since ||r11-5181-g0862d007b564eca8 CC||hubicka at gcc dot gnu.org, ||marxin at gcc dot gnu.org --- Comment #1 from Martin Liška --- Started with r11-5181-g0862d007b564eca8.
