On Wed, Dec 02, 2020 at 09:50:33PM -0500, Jason Merrill wrote:
> On 12/2/20 6:18 PM, Marek Polacek wrote:
> > In this testcase we are crashing trying to gimplify a switch, because
> > the types of the switch condition and case constants have different
> > TYPE_PRECISIONs.
> >
> > This started with my r5-3726 fix: SWITCH_STMT_TYPE is supposed to be the
> > original type of the switch condition before any conversions, so in the
> > C++ FE we need to use unlowered_expr_type to get the unlowered type of
> > enum bit-fields.
> >
> > Normally, the switch type is subject to integral promotions, but here
> > we have a scoped enum type and those don't promote:
> >
> > enum class B { A };
> > struct C { B c : 8; };
> >
> > switch (x.c) // type B
> > case B::A: // type int, will be converted to B
> >
> > Here TREE_TYPE is "signed char" but SWITCH_STMT_TYPE is "B". When
> > gimplifying this in gimplify_switch_expr, the index type is "B" and
> > we convert all the case values to "B" in preprocess_case_label_vec,
> > but SWITCH_COND is of type "signed char": gimple_switch_index should
> > be the (possibly promoted) type, not the original type, so we gimplify
> > the "x.c" SWITCH_COND to a SSA_NAME of type "signed char". And then
> > we crash because the precision of the index type doesn't match the
> > precision of the case value type.
> >
> > I think it makes sense to do the following; at the end of pop_switch
> > we've already issued the switch warnings, and since scoped enums don't
> > promote, it should be okay to use the type of SWITCH_STMT_COND. The
> > r5-3726 change was about giving warnings for enum bit-fields anyway.
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
> >
> > gcc/cp/ChangeLog:
> >
> > PR c++/98043
> > * decl.c (pop_switch): If SWITCH_STMT_TYPE is a scoped enum type,
> > set it to the type of SWITCH_STMT_COND.
>
> It might make sense to do this in cp_genericize_r instead, but here is fine.
Right. In the end I chose pop_switch due to the other SWITCH_STMT_* handling,
so that it's in the same function.
> > --- a/gcc/cp/decl.c
> > +++ b/gcc/cp/decl.c
> > @@ -3711,6 +3711,17 @@ pop_switch (void)
> > SWITCH_STMT_ALL_CASES_P (cs->switch_stmt) = 1;
> > if (!cs->break_stmt_seen_p)
> > SWITCH_STMT_NO_BREAK_P (cs->switch_stmt) = 1;
> > + /* Now that we're done with the switch warnings, set the switch type
> > + to the type of the condition if the index type was of scoped enum
> > type.
> > + (Such types don't participate in the integer promotions.) We do this
> > + because of bit-fields whose declared type is a scoped enum type:
> > + gimplification will use the lowered index type, but convert the
> > + case values to SWITCH_STMT_TYPE, which would have been the declared
> > type
> > + and verify_gimple_switch doesn't accept that. */
> > + if (SWITCH_STMT_TYPE (cs->switch_stmt)
> > + && SCOPED_ENUM_P (SWITCH_STMT_TYPE (cs->switch_stmt)))
> > + SWITCH_STMT_TYPE (cs->switch_stmt)
> > + = TREE_TYPE (SWITCH_STMT_COND (cs->switch_stmt));
>
> What would be the impact of doing this for all
> is_bitfield_expr_with_lowered_type conditions, rather than all scoped enum
> conditions?
The impact is the same: for ordinary bit-fields and unscoped enum bit-fields,
cond will already have been promoted here, so e.g. "(int) a.b", and
is_bitfield_expr_with_lowered_type will return NULL_TREE. And for scoped
enum bit-fields we will do the same thing as in v1. I think the SCOPED_ENUM_P
check is cheaper than is_bitfield_expr_with_lowered_type but I'm fine with
either version. Thanks,
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
-- >8 --
In this testcase we are crashing trying to gimplify a switch, because
the types of the switch condition and case constants have different
TYPE_PRECISIONs.
This started with my r5-3726 fix: SWITCH_STMT_TYPE is supposed to be the
original type of the switch condition before any conversions, so in the
C++ FE we need to use unlowered_expr_type to get the unlowered type of
enum bit-fields.
Normally, the switch type is subject to integral promotions, but here
we have a scoped enum type and those don't promote:
enum class B { A };
struct C { B c : 8; };
switch (x.c) // type B
case B::A: // type int, will be converted to B
Here TREE_TYPE is "signed char" but SWITCH_STMT_TYPE is "B". When
gimplifying this in gimplify_switch_expr, the index type is "B" and
we convert all the case values to "B" in preprocess_case_label_vec,
but SWITCH_COND is of type "signed char": gimple_switch_index should
be the (possibly promoted) type, not the original type, so we gimplify
the "x.c" SWITCH_COND to a SSA_NAME of type "signed char". And then
we crash because the precision of the index type doesn't match the
precision of the case value type.
I think it makes sense to do the following; at the end of pop_switch
we've already issued the switch warnings, and since scoped enums don't
promote, it should be okay to use the type of SWITCH_STMT_COND. The
r5-3726 change was about giving warnings for enum bit-fields anyway.
gcc/cp/ChangeLog:
PR c++/98043
* decl.c (pop_switch): If SWITCH_STMT_TYPE is a scoped enum type,
set it to the type of SWITCH_STMT_COND.
gcc/testsuite/ChangeLog:
PR c++/98043
* g++.dg/cpp0x/enum41.C: New test.
---
gcc/cp/decl.c | 17 +++++++++++----
gcc/testsuite/g++.dg/cpp0x/enum41.C | 32 +++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/enum41.C
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index a28e7924869..7da8c65e984 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -3690,17 +3690,17 @@ void
pop_switch (void)
{
struct cp_switch *cs = switch_stack;
- location_t switch_location;
/* Emit warnings as needed. */
- switch_location = cp_expr_loc_or_input_loc (cs->switch_stmt);
+ location_t switch_location = cp_expr_loc_or_input_loc (cs->switch_stmt);
+ tree cond = SWITCH_STMT_COND (cs->switch_stmt);
const bool bool_cond_p
= (SWITCH_STMT_TYPE (cs->switch_stmt)
&& TREE_CODE (SWITCH_STMT_TYPE (cs->switch_stmt)) == BOOLEAN_TYPE);
if (!processing_template_decl)
c_do_switch_warnings (cs->cases, switch_location,
- SWITCH_STMT_TYPE (cs->switch_stmt),
- SWITCH_STMT_COND (cs->switch_stmt), bool_cond_p);
+ SWITCH_STMT_TYPE (cs->switch_stmt), cond,
+ bool_cond_p);
/* For the benefit of block_may_fallthru remember if the switch body
case labels cover all possible values and if there are break; stmts. */
@@ -3711,6 +3711,15 @@ pop_switch (void)
SWITCH_STMT_ALL_CASES_P (cs->switch_stmt) = 1;
if (!cs->break_stmt_seen_p)
SWITCH_STMT_NO_BREAK_P (cs->switch_stmt) = 1;
+ /* Now that we're done with the switch warnings, set the switch type
+ to the type of the condition if the index type was of scoped enum type.
+ (Such types don't participate in the integer promotions.) We do this
+ because of bit-fields whose declared type is a scoped enum type:
+ gimplification will use the lowered index type, but convert the
+ case values to SWITCH_STMT_TYPE, which would have been the declared type
+ and verify_gimple_switch doesn't accept that. */
+ if (is_bitfield_expr_with_lowered_type (cond))
+ SWITCH_STMT_TYPE (cs->switch_stmt) = TREE_TYPE (cond);
gcc_assert (!cs->in_loop_body_p);
splay_tree_delete (cs->cases);
switch_stack = switch_stack->next;
diff --git a/gcc/testsuite/g++.dg/cpp0x/enum41.C
b/gcc/testsuite/g++.dg/cpp0x/enum41.C
new file mode 100644
index 00000000000..5f6ef13289b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/enum41.C
@@ -0,0 +1,32 @@
+// PR c++/98043
+// { dg-do compile { target c++11 } }
+
+enum class B { A };
+struct C { B c : 8; };
+
+bool
+foo (C x)
+{
+ switch (x.c)
+ {
+ case B::A:
+ return false;
+ default:
+ return true;
+ }
+}
+
+enum E { X };
+struct D { E c : 7; };
+
+bool
+bar (D x)
+{
+ switch (x.c)
+ {
+ case E::X:
+ return false;
+ default:
+ return true;
+ }
+}
base-commit: df933e307b1950ce12472660dcac1765b8eb431d
--
2.28.0
