Hi!
As mentioned in the PR, check_case_bounds warns about case labels on
switches with promoted condition where the values lie fully (or partially
when using ...) outside of the range of the original, non-promoted, type
and adjusts (or doesn't create at all) the CASE_LABEL_EXPRs too early,
so that we don't actually error for duplicate cases.
The following patch defers that warning until later
(c_finish_case/pop_switch), so that we diagnose errors first and only
then adjust/remove them.
Alternatively, I believe we could remove from the patch the in-place
replacement of CASE_LABEL_EXPRs with LABEL_EXPRs if we want to remove,
just splay_tree_remove those, because by my reading of
preprocess_case_label_vec_for_gimple we also ignore the fully out of range
CASE_LABEL_EXPRs there, shall I tweak the patch and rebootstrap?
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-04-09 Jakub Jelinek <ja...@redhat.com>
PR c/89888
* c-common.h (c_add_case_label): Remove orig_type and outside_range_p
arguments.
(c_do_switch_warnings): Remove outside_range_p argument.
* c-common.c (check_case_bounds): Removed.
(c_add_case_label): Remove orig_type and outside_range_p arguments.
Don't call check_case_bounds. Fold low_value as well as high_value.
* c-warn.c (c_do_switch_warnings): Remove outside_range_p argument.
Check for case labels outside of range of original type here and
adjust them or overwrite with LABEL_EXPR.
c/
* c-typeck.c (struct c_switch): Remove outside_range_p member.
(c_start_case): Don't clear it.
(do_case): Adjust c_add_case_label caller.
(c_finish_case): Adjust c_do_switch_warnings caller.
cp/
* decl.c (struct cp_switch): Remove outside_range_p member.
(push_switch): Don't clear it.
(pop_switch): Adjust c_do_switch_warnings caller.
(finish_case_label): Adjust c_add_case_label caller.
testsuite/
* c-c++-common/pr89888.c: New test.
* g++.dg/torture/pr40335.C: Change dg-bogus into dg-warning.
Don't expect -Wswitch-unreachable warning.
--- gcc/c-family/c-common.h.jj 2019-03-20 12:24:57.320308115 +0100
+++ gcc/c-family/c-common.h 2019-04-08 19:02:28.522104090 +0200
@@ -988,8 +988,7 @@ extern tree boolean_increment (enum tree
extern int case_compare (splay_tree_key, splay_tree_key);
-extern tree c_add_case_label (location_t, splay_tree, tree, tree, tree, tree,
- bool *);
+extern tree c_add_case_label (location_t, splay_tree, tree, tree, tree);
extern bool c_switch_covers_all_cases_p (splay_tree, tree);
extern tree build_function_call (location_t, tree, tree);
@@ -1291,8 +1290,7 @@ extern void sizeof_pointer_memaccess_war
bool (*) (tree, tree));
extern void check_main_parameter_types (tree decl);
extern void warnings_for_convert_and_check (location_t, tree, tree, tree);
-extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool,
- bool);
+extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
extern void warn_for_omitted_condop (location_t, tree);
extern bool warn_for_restrict (unsigned, tree *, unsigned);
extern void warn_for_address_or_pointer_of_packed_member (tree, tree);
--- gcc/c-family/c-common.c.jj 2019-03-26 08:52:54.938604825 +0100
+++ gcc/c-family/c-common.c 2019-04-09 01:28:01.199754481 +0200
@@ -314,8 +314,6 @@ const struct fname_var_t fname_vars[] =
struct visibility_flags visibility_options;
static tree check_case_value (location_t, tree);
-static bool check_case_bounds (location_t, tree, tree, tree *, tree *,
- bool *);
static void check_nonnull_arg (void *, tree, unsigned HOST_WIDE_INT);
@@ -2103,86 +2101,6 @@ check_case_value (location_t loc, tree v
return value;
}
-/* See if the case values LOW and HIGH are in the range of the original
- type (i.e. before the default conversion to int) of the switch testing
- expression.
- TYPE is the promoted type of the testing expression, and ORIG_TYPE is
- the type before promoting it. CASE_LOW_P is a pointer to the lower
- bound of the case label, and CASE_HIGH_P is the upper bound or NULL
- if the case is not a case range.
- The caller has to make sure that we are not called with NULL for
- CASE_LOW_P (i.e. the default case). OUTSIDE_RANGE_P says whether there
- was a case value that doesn't fit into the range of the ORIG_TYPE.
- Returns true if the case label is in range of ORIG_TYPE (saturated or
- untouched) or false if the label is out of range. */
-
-static bool
-check_case_bounds (location_t loc, tree type, tree orig_type,
- tree *case_low_p, tree *case_high_p,
- bool *outside_range_p)
-{
- tree min_value, max_value;
- tree case_low = *case_low_p;
- tree case_high = case_high_p ? *case_high_p : case_low;
-
- /* If there was a problem with the original type, do nothing. */
- if (orig_type == error_mark_node)
- return true;
-
- min_value = TYPE_MIN_VALUE (orig_type);
- max_value = TYPE_MAX_VALUE (orig_type);
-
- /* We'll really need integer constants here. */
- case_low = fold (case_low);
- case_high = fold (case_high);
-
- /* Case label is less than minimum for type. */
- if (tree_int_cst_compare (case_low, min_value) < 0
- && tree_int_cst_compare (case_high, min_value) < 0)
- {
- warning_at (loc, 0, "case label value is less than minimum value "
- "for type");
- *outside_range_p = true;
- return false;
- }
-
- /* Case value is greater than maximum for type. */
- if (tree_int_cst_compare (case_low, max_value) > 0
- && tree_int_cst_compare (case_high, max_value) > 0)
- {
- warning_at (loc, 0, "case label value exceeds maximum value for type");
- *outside_range_p = true;
- return false;
- }
-
- /* Saturate lower case label value to minimum. */
- if (tree_int_cst_compare (case_high, min_value) >= 0
- && tree_int_cst_compare (case_low, min_value) < 0)
- {
- warning_at (loc, 0, "lower value in case label range"
- " less than minimum value for type");
- *outside_range_p = true;
- case_low = min_value;
- }
-
- /* Saturate upper case label value to maximum. */
- if (tree_int_cst_compare (case_low, max_value) <= 0
- && tree_int_cst_compare (case_high, max_value) > 0)
- {
- warning_at (loc, 0, "upper value in case label range"
- " exceeds maximum value for type");
- *outside_range_p = true;
- case_high = max_value;
- }
-
- if (*case_low_p != case_low)
- *case_low_p = convert (type, case_low);
- if (case_high_p && *case_high_p != case_high)
- *case_high_p = convert (type, case_high);
-
- return true;
-}
-
/* Return an integer type with BITS bits of precision,
that is unsigned if UNSIGNEDP is nonzero, otherwise signed. */
@@ -4873,13 +4791,12 @@ case_compare (splay_tree_key k1, splay_t
usual C/C++ syntax, rather than the GNU case range extension.
CASES is a tree containing all the case ranges processed so far;
COND is the condition for the switch-statement itself.
- OUTSIDE_RANGE_P says whether there was a case value that doesn't
- fit into the range of the ORIG_TYPE. Returns the CASE_LABEL_EXPR
- created, or ERROR_MARK_NODE if no CASE_LABEL_EXPR is created. */
+ Returns the CASE_LABEL_EXPR created, or ERROR_MARK_NODE if no
+ CASE_LABEL_EXPR is created. */
tree
-c_add_case_label (location_t loc, splay_tree cases, tree cond, tree orig_type,
- tree low_value, tree high_value, bool *outside_range_p)
+c_add_case_label (location_t loc, splay_tree cases, tree cond,
+ tree low_value, tree high_value)
{
tree type;
tree label;
@@ -4913,6 +4830,7 @@ c_add_case_label (location_t loc, splay_
{
low_value = check_case_value (loc, low_value);
low_value = convert_and_check (loc, type, low_value);
+ low_value = fold (low_value);
if (low_value == error_mark_node)
goto error_out;
}
@@ -4920,6 +4838,7 @@ c_add_case_label (location_t loc, splay_
{
high_value = check_case_value (loc, high_value);
high_value = convert_and_check (loc, type, high_value);
+ high_value = fold (high_value);
if (high_value == error_mark_node)
goto error_out;
}
@@ -4935,15 +4854,6 @@ c_add_case_label (location_t loc, splay_
warning_at (loc, 0, "empty range specified");
}
- /* See if the case is in range of the type of the original testing
- expression. If both low_value and high_value are out of range,
- don't insert the case label and return NULL_TREE. */
- if (low_value
- && !check_case_bounds (loc, type, orig_type,
- &low_value, high_value ? &high_value : NULL,
- outside_range_p))
- return NULL_TREE;
-
/* Look up the LOW_VALUE in the table of case labels we already
have. */
node = splay_tree_lookup (cases, (splay_tree_key) low_value);
--- gcc/c-family/c-warn.c.jj 2019-04-08 10:11:26.648251537 +0200
+++ gcc/c-family/c-warn.c 2019-04-09 01:30:35.157348989 +0200
@@ -1428,12 +1428,103 @@ match_case_to_enum (splay_tree_node node
void
c_do_switch_warnings (splay_tree cases, location_t switch_location,
- tree type, tree cond, bool bool_cond_p,
- bool outside_range_p)
+ tree type, tree cond, bool bool_cond_p)
{
splay_tree_node default_node;
splay_tree_node node;
tree chain;
+ bool outside_range_p = false;
+
+ if (type != error_mark_node
+ && type != TREE_TYPE (cond)
+ && INTEGRAL_TYPE_P (type)
+ && INTEGRAL_TYPE_P (TREE_TYPE (cond))
+ && (!tree_int_cst_equal (TYPE_MIN_VALUE (type),
+ TYPE_MIN_VALUE (TREE_TYPE (cond)))
+ || !tree_int_cst_equal (TYPE_MAX_VALUE (type),
+ TYPE_MAX_VALUE (TREE_TYPE (cond)))))
+ {
+ tree min_value = TYPE_MIN_VALUE (type);
+ tree max_value = TYPE_MAX_VALUE (type);
+
+ node = splay_tree_predecessor (cases, (splay_tree_key) min_value);
+ if (node && node->key)
+ {
+ outside_range_p = true;
+ /* There is at least one case smaller than TYPE's minimum value.
+ NODE itself could be still a range overlapping the valid values,
+ but any predecessors thereof except the default case will be
+ completely outside of range. */
+ if (CASE_HIGH ((tree) node->value)
+ && tree_int_cst_compare (CASE_HIGH ((tree) node->value),
+ min_value) >= 0)
+ {
+ location_t loc = EXPR_LOCATION ((tree) node->value);
+ warning_at (loc, 0, "lower value in case label range"
+ " less than minimum value for type");
+ CASE_LOW ((tree) node->value) = convert (TREE_TYPE (cond),
+ min_value);
+ node->key = (splay_tree_key) CASE_LOW ((tree) node->value);
+ }
+ /* All the following ones are completely outside of range. */
+ do
+ {
+ node = splay_tree_predecessor (cases,
+ (splay_tree_key) min_value);
+ if (node == NULL || !node->key)
+ break;
+ location_t loc = EXPR_LOCATION ((tree) node->value);
+ warning_at (loc, 0, "case label value is less than minimum "
+ "value for type");
+ tree case_expr = (tree) node->value;
+ splay_tree_remove (cases, node->key);
+ tree label = CASE_LABEL (case_expr);
+ /* Turn the CASE_LABEL_EXPR into LABEL_EXPR. */
+ gcc_assert (CASE_CHAIN (case_expr) == NULL_TREE);
+ CASE_LOW_SEEN (case_expr) = 0;
+ CASE_HIGH_SEEN (case_expr) = 0;
+ TREE_SET_CODE (case_expr, LABEL_EXPR);
+ LABEL_EXPR_LABEL (case_expr) = label;
+ }
+ while (1);
+ }
+ node = splay_tree_lookup (cases, (splay_tree_key) max_value);
+ if (node == NULL)
+ node = splay_tree_predecessor (cases, (splay_tree_key) max_value);
+ /* Handle a single node that might partially overlap the range. */
+ if (node
+ && node->key
+ && CASE_HIGH ((tree) node->value)
+ && tree_int_cst_compare (CASE_HIGH ((tree) node->value),
+ max_value) > 0)
+ {
+ location_t loc = EXPR_LOCATION ((tree) node->value);
+ warning_at (loc, 0, "upper value in case label range"
+ " exceeds maximum value for type");
+ CASE_HIGH ((tree) node->value)
+ = convert (TREE_TYPE (cond), max_value);
+ outside_range_p = true;
+ }
+ /* And any nodes that are completely outside of the range. */
+ while ((node = splay_tree_successor (cases,
+ (splay_tree_key) max_value))
+ != NULL)
+ {
+ location_t loc = EXPR_LOCATION ((tree) node->value);
+ warning_at (loc, 0,
+ "case label value exceeds maximum value for type");
+ tree case_expr = (tree) node->value;
+ splay_tree_remove (cases, node->key);
+ tree label = CASE_LABEL (case_expr);
+ /* Turn the CASE_LABEL_EXPR into LABEL_EXPR. */
+ gcc_assert (CASE_CHAIN (case_expr) == NULL_TREE);
+ CASE_LOW_SEEN (case_expr) = 0;
+ CASE_HIGH_SEEN (case_expr) = 0;
+ TREE_SET_CODE (case_expr, LABEL_EXPR);
+ LABEL_EXPR_LABEL (case_expr) = label;
+ outside_range_p = true;
+ }
+ }
if (!warn_switch && !warn_switch_enum && !warn_switch_default
&& !warn_switch_bool)
--- gcc/c/c-typeck.c.jj 2019-02-28 08:17:03.044512874 +0100
+++ gcc/c/c-typeck.c 2019-04-08 19:02:47.371799493 +0200
@@ -10686,10 +10686,6 @@ struct c_switch {
/* Remember whether the controlling expression had boolean type
before integer promotions for the sake of -Wswitch-bool. */
bool bool_cond_p;
-
- /* Remember whether there was a case value that is outside the
- range of the ORIG_TYPE. */
- bool outside_range_p;
};
/* A stack of the currently active switch statements. The innermost
@@ -10766,7 +10762,6 @@ c_start_case (location_t switch_loc,
cs->cases = splay_tree_new (case_compare, NULL, NULL);
cs->bindings = c_get_switch_bindings ();
cs->bool_cond_p = bool_cond_p;
- cs->outside_range_p = false;
cs->next = c_switch_stack;
c_switch_stack = cs;
@@ -10812,9 +10807,7 @@ do_case (location_t loc, tree low_value,
label = c_add_case_label (loc, c_switch_stack->cases,
SWITCH_COND (c_switch_stack->switch_expr),
- c_switch_stack->orig_type,
- low_value, high_value,
- &c_switch_stack->outside_range_p);
+ low_value, high_value);
if (label == error_mark_node)
label = NULL_TREE;
return label;
@@ -10835,8 +10828,7 @@ c_finish_case (tree body, tree type)
switch_location = EXPR_LOCATION (cs->switch_expr);
c_do_switch_warnings (cs->cases, switch_location,
type ? type : TREE_TYPE (cs->switch_expr),
- SWITCH_COND (cs->switch_expr),
- cs->bool_cond_p, cs->outside_range_p);
+ SWITCH_COND (cs->switch_expr), cs->bool_cond_p);
if (c_switch_covers_all_cases_p (cs->cases, TREE_TYPE (cs->switch_expr)))
SWITCH_ALL_CASES_P (cs->switch_expr) = 1;
--- gcc/cp/decl.c.jj 2019-04-08 10:11:28.185226285 +0200
+++ gcc/cp/decl.c 2019-04-08 19:03:26.186172295 +0200
@@ -3475,9 +3475,6 @@ struct cp_switch
label. We need a tree, rather than simply a hash table, because
of the GNU case range extension. */
splay_tree cases;
- /* Remember whether there was a case value that is outside the
- range of the original type of the controlling expression. */
- bool outside_range_p;
/* Remember whether a default: case label has been seen. */
bool has_default_p;
/* Remember whether a BREAK_STMT has been seen in this SWITCH_STMT. */
@@ -3506,7 +3503,6 @@ push_switch (tree switch_stmt)
p->next = switch_stack;
p->switch_stmt = switch_stmt;
p->cases = splay_tree_new (case_compare, NULL, NULL);
- p->outside_range_p = false;
p->has_default_p = false;
p->break_stmt_seen_p = false;
p->in_loop_body_p = false;
@@ -3527,8 +3523,7 @@ pop_switch (void)
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, cs->outside_range_p);
+ SWITCH_STMT_COND (cs->switch_stmt), 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. */
@@ -3643,9 +3638,7 @@ finish_case_label (location_t loc, tree
low_value = case_conversion (type, low_value);
high_value = case_conversion (type, high_value);
- r = c_add_case_label (loc, switch_stack->cases, cond, type,
- low_value, high_value,
- &switch_stack->outside_range_p);
+ r = c_add_case_label (loc, switch_stack->cases, cond, low_value, high_value);
/* After labels, make any new cleanups in the function go into their
own new (temporary) binding contour. */
--- gcc/testsuite/c-c++-common/pr89888.c.jj 2019-04-08 19:54:37.619554189
+0200
+++ gcc/testsuite/c-c++-common/pr89888.c 2019-04-08 20:28:41.390495161
+0200
@@ -0,0 +1,67 @@
+/* PR c/89888 */
+/* { dg-do compile { target { int32 } } } */
+/* { dg-options "" } */
+
+long long y;
+
+void
+foo (unsigned char x)
+{
+ switch (x)
+ {
+ case -1: y = -1; break; /* { dg-message "previously
used here" } */
+ /* { dg-warning "case label
value is less than minimum value for type" "" { target *-*-* } .-1 } */
+ case 0xffffffff: y = 0xffffffff; break; /* { dg-error "duplicate case
value" } */
+ case ~0U: y = ~0U; break; /* { dg-error "duplicate case
value" } */
+ }
+}
+
+void
+bar (unsigned char x)
+{
+ switch (x)
+ {
+ case -1: y = -1; break; /* { dg-message "previously
used here" } */
+ /* { dg-warning "case label
value is less than minimum value for type" "" { target *-*-* } .-1 } */
+ case -1: y = -1; break; /* { dg-error "duplicate case
value" } */
+ case -1: y = -1; break; /* { dg-error "duplicate case
value" } */
+ }
+}
+
+void
+baz (unsigned char x)
+{
+ switch (x)
+ {
+ case -7: y = -7; break; /* { dg-warning "case label
value is less than minimum value for type" } */
+ case -5 ... 2: y = -5; break; /* { dg-warning "lower value in
case label range less than minimum value for type" } */
+ case 18: y = 18; break;
+ case (unsigned char) -2 ... 4 + (unsigned char) -2: y = 2; break; /* {
dg-warning "upper value in case label range exceeds maximum value for type" } */
+ case 24 + (unsigned char) -2: y = 3; break; /* { dg-warning "case
label value exceeds maximum value for type" } */
+ }
+}
+
+void
+qux (unsigned char x)
+{
+ switch (x)
+ {
+ case (unsigned char) -1 ... 1 + (unsigned char) -1: y = 2; break; /* {
dg-warning "upper value in case label range exceeds maximum value for type" } */
+ case -12: y = -7; break; /* { dg-warning "case label
value is less than minimum value for type" } */
+ case 18: y = 18; break;
+ case 27 + (unsigned char) -1: y = 3; break; /* { dg-warning "case
label value exceeds maximum value for type" } */
+ case -1 ... 0: y = -5; break; /* { dg-warning "lower value in
case label range less than minimum value for type" } */
+ }
+}
+
+void
+quux (unsigned char x)
+{
+ switch (x)
+ {
+ case (unsigned char) -2 ... (unsigned char) -1: y = 2; break;
+ case 18: y = 18; break;
+ case 1 + (unsigned char) -1: y = 3; break; /* { dg-warning "case label
value exceeds maximum value for type" } */
+ case -2 ... -1: y = -5; break; /* { dg-warning "case label
value is less than minimum value for type" } */
+ }
+}
--- gcc/testsuite/g++.dg/torture/pr40335.C.jj 2016-05-24 10:55:59.788180310
+0200
+++ gcc/testsuite/g++.dg/torture/pr40335.C 2019-04-09 01:35:39.138513488
+0200
@@ -7,8 +7,8 @@ main (void)
int i = -1;
switch ((signed char) i)
{
- case 255: /* { dg-bogus "exceeds maximum value" "" { xfail *-*-* } } */
- abort (); /* { dg-warning "statement will never be executed" } */
+ case 255: /* { dg-warning "exceeds maximum value" } */
+ abort ();
default:
break;
}
Jakub
