+2015-06-29 Marek Polacek <polacek@redhat.com>
+
+ PR c/66322
+ * function.c (stack_protect_epilogue): Remove a cast to int.
+ * doc/invoke.texi: Update -Wswitch-bool description.
+
2015-06-29 Richard Biener <rguenther@suse.de>
* genmatch.c (add_operator): Treat ADDR_EXPR as atom.
+2015-06-29 Marek Polacek <polacek@redhat.com>
+
+ PR c/66322
+ * c-common.c (check_case_bounds): Add bool * parameter. Set
+ OUTSIDE_RANGE_P.
+ (c_add_case_label): Add bool * parameter. Pass it down to
+ check_case_bounds.
+ (c_do_switch_warnings): Add bool parameters. Implement -Wswitch-bool
+ warning here.
+ * c-common.h (c_add_case_label, c_do_switch_warnings): Update
+ declarations.
+
2015-06-27 Marek Polacek <polacek@redhat.com>
* c-common.c (check_main_parameter_types): Use VECTOR_TYPE_P
static tree c_fully_fold_internal (tree expr, bool, bool *, bool *, bool);
static tree check_case_value (location_t, tree);
-static bool check_case_bounds (location_t, tree, tree, tree *, tree *);
+static bool check_case_bounds (location_t, tree, tree, tree *, tree *,
+ bool *);
static tree handle_packed_attribute (tree *, tree, tree, int, bool *);
static tree handle_nocommon_attribute (tree *, tree, tree, int, bool *);
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).
+ 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)
+ tree *case_low_p, tree *case_high_p,
+ bool *outside_range_p)
{
tree min_value, max_value;
tree case_low = *case_low_p;
{
warning_at (loc, 0, "case label value is less than minimum value "
"for type");
+ *outside_range_p = true;
return false;
}
&& 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;
}
{
warning_at (loc, 0, "lower value in case label range"
" less than minimum value for type");
+ *outside_range_p = true;
case_low = min_value;
}
{
warning_at (loc, 0, "upper value in case label range"
" exceeds maximum value for type");
+ *outside_range_p = true;
case_high = max_value;
}
HIGH_VALUE is NULL_TREE, then case label was declared using the
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. Returns the
- CASE_LABEL_EXPR created, or ERROR_MARK_NODE if no CASE_LABEL_EXPR
- is created. */
+ 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. */
tree
c_add_case_label (location_t loc, splay_tree cases, tree cond, tree orig_type,
- tree low_value, tree high_value)
+ tree low_value, tree high_value, bool *outside_range_p)
{
tree type;
tree label;
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))
+ &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
void
c_do_switch_warnings (splay_tree cases, location_t switch_location,
- tree type, tree cond)
+ tree type, tree cond, bool bool_cond_p,
+ bool outside_range_p)
{
splay_tree_node default_node;
splay_tree_node node;
tree chain;
- if (!warn_switch && !warn_switch_enum && !warn_switch_default)
+ if (!warn_switch && !warn_switch_enum && !warn_switch_default
+ && !warn_switch_bool)
return;
default_node = splay_tree_lookup (cases, (splay_tree_key) NULL);
warning_at (switch_location, OPT_Wswitch_default,
"switch missing default case");
+ /* There are certain cases where -Wswitch-bool warnings aren't
+ desirable, such as
+ switch (boolean)
+ {
+ case true: ...
+ case false: ...
+ }
+ so be careful here. */
+ if (warn_switch_bool && bool_cond_p)
+ {
+ splay_tree_node min_node;
+ /* If there's a default node, it's also the value with the minimal
+ key. So look at the penultimate key (if any). */
+ if (default_node)
+ min_node = splay_tree_successor (cases, (splay_tree_key) NULL);
+ else
+ min_node = splay_tree_min (cases);
+ tree min = min_node ? (tree) min_node->key : NULL_TREE;
+
+ splay_tree_node max_node = splay_tree_max (cases);
+ /* This might be a case range, so look at the value with the
+ maximal key and then check CASE_HIGH. */
+ tree max = max_node ? (tree) max_node->value : NULL_TREE;
+ if (max)
+ max = CASE_HIGH (max) ? CASE_HIGH (max) : CASE_LOW (max);
+
+ /* If there's a case value > 1 or < 0, that is outside bool
+ range, warn. */
+ if (outside_range_p
+ || (max && wi::gts_p (max, 1))
+ || (min && wi::lts_p (min, 0))
+ /* And handle the
+ switch (boolean)
+ {
+ case true: ...
+ case false: ...
+ default: ...
+ }
+ case, where we want to warn. */
+ || (default_node
+ && max && wi::eq_p (max, 1)
+ && min && wi::eq_p (min, 0)))
+ warning_at (switch_location, OPT_Wswitch_bool,
+ "switch condition has boolean value");
+ }
+
/* From here on, we only care about about enumerated types. */
if (!type || TREE_CODE (type) != ENUMERAL_TYPE)
return;
extern int case_compare (splay_tree_key, splay_tree_key);
-extern tree c_add_case_label (location_t, splay_tree, tree, tree, tree, tree);
+extern tree c_add_case_label (location_t, splay_tree, tree, tree, tree, tree,
+ bool *);
-extern void c_do_switch_warnings (splay_tree, location_t, tree, tree);
+extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool,
+ bool);
extern tree build_function_call (location_t, tree, tree);
+2015-06-29 Marek Polacek <polacek@redhat.com>
+
+ PR c/66322
+ * c-typeck.c (struct c_switch): Add BOOL_COND_P and OUTSIDE_RANGE_P.
+ (c_start_case): Set BOOL_COND_P and OUTSIDE_RANGE_P. Don't warn
+ about -Wswitch-bool here.
+ (do_case): Update c_add_case_label call.
+ (c_finish_case): Update c_do_switch_warnings call.
+
2015-06-27 Marek Polacek <polacek@redhat.com>
* c-typeck.c: Use VECTOR_TYPE_P throughout.
/* The next node on the stack. */
struct c_switch *next;
+
+ /* 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
/* Start a C switch statement, testing expression EXP. Return the new
SWITCH_EXPR. SWITCH_LOC is the location of the `switch'.
SWITCH_COND_LOC is the location of the switch's condition.
- EXPLICIT_CAST_P is true if the expression EXP has explicit cast. */
+ EXPLICIT_CAST_P is true if the expression EXP has an explicit cast. */
tree
c_start_case (location_t switch_loc,
tree exp, bool explicit_cast_p)
{
tree orig_type = error_mark_node;
+ bool bool_cond_p = false;
struct c_switch *cs;
if (exp != error_mark_node)
/* Explicit cast to int suppresses this warning. */
&& !(TREE_CODE (type) == INTEGER_TYPE
&& explicit_cast_p))
- warning_at (switch_cond_loc, OPT_Wswitch_bool,
- "switch condition has boolean value");
+ bool_cond_p = true;
if (!in_system_header_at (input_location)
&& (type == long_integer_type_node
cs->orig_type = orig_type;
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;
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);
+ low_value, high_value,
+ &c_switch_stack->outside_range_p);
if (label == error_mark_node)
label = NULL_TREE;
return label;
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));
+ SWITCH_COND (cs->switch_expr),
+ cs->bool_cond_p, cs->outside_range_p);
/* Pop the stack. */
c_switch_stack = cs->next;
+2015-06-29 Marek Polacek <polacek@redhat.com>
+
+ PR c/66322
+ * decl.c (struct cp_switch): Add OUTSIDE_RANGE_P.
+ (push_switch): Set OUTSIDE_RANGE_P.
+ (pop_switch): Update c_do_switch_warnings call.
+ (finish_case_label): Update c_add_case_label call.
+ * semantics.c (finish_switch_cond): Don't warn about -Wswitch-bool
+ here.
+
2015-06-27 Marek Polacek <polacek@redhat.com>
* call.c: Use VECTOR_TYPE_P.
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;
};
/* A stack of the currently active switch statements. The innermost
p->next = switch_stack;
p->switch_stmt = switch_stmt;
p->cases = splay_tree_new (case_compare, NULL, NULL);
+ p->outside_range_p = false;
switch_stack = p;
}
/* Emit warnings as needed. */
switch_location = EXPR_LOC_OR_LOC (cs->switch_stmt, input_location);
+ 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));
+ SWITCH_STMT_COND (cs->switch_stmt),
+ bool_cond_p, cs->outside_range_p);
splay_tree_delete (cs->cases);
switch_stack = switch_stack->next;
high_value = case_conversion (type, high_value);
r = c_add_case_label (loc, switch_stack->cases, cond, type,
- low_value, high_value);
+ low_value, high_value,
+ &switch_stack->outside_range_p);
/* After labels, make any new cleanups in the function go into their
own new (temporary) binding contour. */
orig_type = TREE_TYPE (cond);
if (cond != error_mark_node)
{
- /* Warn if the condition has boolean value. */
- if (TREE_CODE (orig_type) == BOOLEAN_TYPE)
- warning_at (input_location, OPT_Wswitch_bool,
- "switch condition has type bool");
-
/* [stmt.switch]
Integral promotions are performed. */
@item -Wswitch-bool
@opindex Wswitch-bool
@opindex Wno-switch-bool
-Warn whenever a @code{switch} statement has an index of boolean type.
+Warn whenever a @code{switch} statement has an index of boolean type
+and the case values are outside the range of a boolean type.
It is possible to suppress this warning by casting the controlling
expression to a type other than @code{bool}. For example:
@smallexample
/* Allow the target to compare Y with X without leaking either into
a register. */
- switch ((int) (HAVE_stack_protect_test != 0))
+ switch (HAVE_stack_protect_test != 0)
{
case 1:
tmp = gen_stack_protect_test (x, y, label);
+2015-06-29 Marek Polacek <polacek@redhat.com>
+
+ PR c/66322
+ * c-c++-common/pr60439.c: Add dg-prune-output and add switch cases.
+ * c-c++-common/pr66322.c: New test.
+ * g++.dg/eh/scope1.C: Remove dg-warning.
+
2015-06-29 Richard Biener <rguenther@suse.de>
PR tree-optimization/66677
/* PR c/60439 */
/* { dg-do compile } */
+/* { dg-prune-output "case label value exceeds" } */
#ifndef __cplusplus
# define bool _Bool
f1 (bool b)
{
switch (b) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
}
void
f2 (int a, int b)
{
switch (a && b) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch ((bool) (a && b)) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch ((a && b) || a) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
/* No warnings on following. */
switch ((int) (a && b))
break;
f3 (int a)
{
switch (!!a) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (!a) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
}
void
f4 (void)
{
switch (foo ()) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
}
void
f5 (int a)
{
switch (a == 3) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (a != 3) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (a > 3) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (a < 3) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (a <= 3) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (a >= 3) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (foo (), foo (), a >= 42) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (a == 3, a & 4, a ^ 5, a)
break;
switch ((int) (a == 3))
f6 (bool b)
{
switch (b) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (!b) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
switch (b++) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
}
void
{
bool b;
switch (b = 1) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 3:
+ break;
+ }
}
void
switch ((unsigned int) i)
break;
switch ((bool) i) /* { dg-warning "switch condition has" } */
- break;
+ {
+ case 11:
+ break;
+ }
}
--- /dev/null
+/* PR c/66322 */
+/* { dg-do compile } */
+
+#ifndef __cplusplus
+# define bool _Bool
+# define true 1
+# define false 0
+#endif
+
+void
+nowarn (bool b)
+{
+ switch (b)
+ ;
+
+ switch (b)
+ {
+ case true:
+ case false:
+ break;
+ }
+
+ switch (b)
+ {
+ case true:
+ break;
+ }
+
+ switch (b)
+ {
+ case true:
+ default:
+ break;
+ }
+
+ switch (b)
+ {
+ case false:
+ break;
+ }
+
+ switch (b)
+ {
+ case false:
+ default:
+ break;
+ }
+
+ switch (b)
+ {
+ default:
+ break;
+ }
+
+ switch (b)
+ {
+ case false ... true:
+ break;
+ }
+
+ switch (b)
+ {
+ case 1:
+ switch (b)
+ {
+ case true:
+ default:
+ break;
+ }
+ default:
+ break;
+ }
+}
+
+void
+warn (bool b)
+{
+ switch (b) /* { dg-warning "switch condition has" } */
+ {
+ case true:
+ case false:
+ default:
+ break;
+ }
+
+ switch (b) /* { dg-warning "switch condition has" } */
+ {
+ case false ... true:
+ default:
+ break;
+ }
+}
+
+void
+warn2 (int n)
+{
+ switch (n == 2) /* { dg-warning "switch condition has" } */
+ {
+ case 0 ... 2: /* { dg-warning "upper value" "" { target c++ } } */
+ default:
+ break;
+ }
+
+ switch (n == 2) /* { dg-warning "switch condition has" } */
+ {
+ case 1 ... 10: /* { dg-warning "upper value" "" { target c++ } } */
+ default:
+ break;
+ }
+
+ switch (n == 2) /* { dg-warning "switch condition has" } */
+ {
+ case 2: /* { dg-warning "case label" "" { target c++ } } */
+ break;
+ }
+
+ switch (n == 2) /* { dg-warning "switch condition has" } */
+ {
+ case 0:
+ case 1:
+ case -1: /* { dg-warning "case label" "" { target c++ } } */
+ break;
+ }
+
+ switch (n == 2) /* { dg-warning "switch condition has" } */
+ {
+ case -1 ... 1: /* { dg-warning "lower value" "" { target c++ } } */
+ break;
+ }
+
+ switch (n == 2) /* { dg-warning "switch condition has" } */
+ {
+ case -1 ... 0: /* { dg-warning "lower value" "" { target c++ } } */
+ default:
+ break;
+ }
+
+ switch (n == 2) /* { dg-warning "switch condition has" } */
+ {
+ case -10 ... -1: /* { dg-warning "case label" "" { target c++ } } */
+ default:
+ break;
+ }
+}
void f4 ()
{
- switch (C br = C()) /* { dg-warning "switch condition has" } */
+ switch (C br = C())
{
default:
abort ();