- implemented -Wswitch-enum

- revert wrong typo fix
- fixed reachability test by using first_case and last_case

[r21577]

diff --git a/TODO b/TODO
index 7bcd13d..bffb3bd 100644 (file)
--- a/TODO
+++ b/TODO
@@ -33,7 +33,6 @@ Missing Errors:
 
 Missing Warnings:
 - dead assignments (int x = 5; x = bla(); -> dead assignment at x = 5;)
-- check switches for all enums values
 - catch the if(k = b) cases, maybe require all assignments to be in parentheses
   (but some few exceptions like toplevel, nested assignments)
 

diff --git a/ast2firm.c b/ast2firm.c
index 839f75a..6735e32 100644 (file)
--- a/ast2firm.c
+++ b/ast2firm.c
@@ -4462,7 +4462,7 @@ static void switch_statement_to_firm(switch_statement_t *statement)
        } else {
                ++def_nr;
        }
-       statement->def_proj_nr = def_nr;
+       statement->default_proj_nr = def_nr;
 
        if (statement->body != NULL) {
                statement_to_firm(statement->body);
@@ -4476,7 +4476,7 @@ static void switch_statement_to_firm(switch_statement_t *statement)
        if (!saw_default_label) {
                set_cur_block(get_nodes_block(cond));
                ir_node *const proj = new_d_defaultProj(dbgi, cond,
-                                                       statement->def_proj_nr);
+                                                       statement->default_proj_nr);
                add_immBlock_pred(get_break_label(), proj);
        }
 
@@ -4518,7 +4518,7 @@ static void case_label_to_firm(const case_label_statement_t *statement)
        } else {
                saw_default_label = true;
                proj = new_d_defaultProj(dbgi, current_switch_cond,
-                                        current_switch->def_proj_nr);
+                                        current_switch->default_proj_nr);
 
                add_immBlock_pred(block, proj);
        }

diff --git a/ast_t.h b/ast_t.h
index 7127c3f..4c4cf92 100644 (file)
--- a/ast_t.h
+++ b/ast_t.h
@@ -555,7 +555,7 @@ struct declaration_t {
        symbol_t           *symbol;
        source_position_t   source_position;
        union {
-               bool            complete;           /**< used to indicate wether struct/union types are already defined or if just the name is declared */
+               bool            complete;           /**< used to indicate whether struct/union types are already defined or if just the name is declared */
                statement_t    *statement;
                initializer_t  *initializer;
                expression_t   *enum_value;
@@ -649,8 +649,9 @@ struct switch_statement_t {
        statement_base_t        base;
        expression_t           *expression;
        statement_t            *body;
-       case_label_statement_t *first_case, *last_case;
-       unsigned long           def_proj_nr;  /**< the Proj-number for the default Proj. */
+       case_label_statement_t *first_case, *last_case;  /**< List of all cases, including default. */
+       case_label_statement_t *default_label;    /**< The default label if existent. */
+       unsigned long           default_proj_nr;  /**< The Proj-number for the default Proj. */
 };
 
 struct goto_statement_t {

diff --git a/parser.c b/parser.c
index f41ed52..f9920b8 100644 (file)
--- a/parser.c
+++ b/parser.c
@@ -3233,7 +3233,7 @@ static void parse_declaration_specifiers(declaration_specifiers_t *specifiers)
 
                        type_t *const typedef_type = get_typedef_type(token.v.symbol);
                        if (typedef_type == NULL) {
-                               /* Be somewhat resilient to typos like 'void f()' at the beginning of a
+                               /* Be somewhat resilient to typos like 'vodi f()' at the beginning of a
                                 * declaration, so it doesn't generate 'implicit int' followed by more
                                 * errors later on. */
                                token_type_t const la1_type = (token_type_t)look_ahead(1)->type;
@@ -4786,9 +4786,7 @@ static void check_reachable(statement_t *const stmt)
                                                continue;
                                        }
 
-                                       expression_t *const case_expr = i->expression;
-                                       if (is_constant_expression(case_expr) &&
-                                           fold_constant(case_expr) == val) {
+                                       if (i->first_case <= val && val <= i->last_case) {
                                                check_reachable((statement_t*)i);
                                                return;
                                        }
@@ -8266,20 +8264,6 @@ end_error:
        return create_invalid_statement();
 }
 
-/**
- * Finds an existing default label of a switch statement.
- */
-static case_label_statement_t *
-find_default_label(const switch_statement_t *statement)
-{
-       case_label_statement_t *label = statement->first_case;
-       for ( ; label != NULL; label = label->next) {
-               if (label->expression == NULL)
-                       return label;
-       }
-       return NULL;
-}
-
 /**
  * Parse a default statement.
  */
@@ -8294,11 +8278,13 @@ static statement_t *parse_default_statement(void)
 
        expect(':');
        if (current_switch != NULL) {
-               const case_label_statement_t *def_label = find_default_label(current_switch);
+               const case_label_statement_t *def_label = current_switch->default_label;
                if (def_label != NULL) {
                        errorf(HERE, "multiple default labels in one switch (previous declared %P)",
                               &def_label->base.source_position);
                } else {
+                       current_switch->default_label = &statement->case_label;
+
                        /* link all cases into the switch statement */
                        if (current_switch->last_case == NULL) {
                                current_switch->first_case      = &statement->case_label;
@@ -8449,6 +8435,46 @@ end_error:
        return create_invalid_statement();
 }
 
+/**
+ * Check that all enums are handled in a switch.
+ *
+ * @param statement  the switch statement to check
+ */
+static void check_enum_cases(const switch_statement_t *statement) {
+       const type_t *type = skip_typeref(statement->expression->base.type);
+       if (! is_type_enum(type))
+               return;
+       const enum_type_t *enumt = &type->enumt;
+
+       /* if we have a default, no warnings */
+       if (statement->default_label != NULL)
+               return;
+
+       /* FIXME: calculation of value should be done while parsing */
+       const declaration_t *declaration;
+       long last_value = -1;
+       for (declaration = enumt->declaration->next;
+            declaration != NULL && declaration->storage_class == STORAGE_CLASS_ENUM_ENTRY;
+                declaration = declaration->next) {
+               const expression_t *expression = declaration->init.enum_value;
+               long                value      = expression != NULL ? fold_constant(expression) : last_value + 1;
+               bool                found      = false;
+               for (const case_label_statement_t *l = statement->first_case; l != NULL; l = l->next) {
+                       if (l->expression == NULL)
+                               continue;
+                       if (l->first_case <= value && value <= l->last_case) {
+                               found = true;
+                               break;
+                       }
+               }
+               if (! found) {
+                       warningf(&statement->base.source_position,
+                               "enumeration value '%Y' not handled in switch", declaration->symbol);
+               }
+               last_value = value;
+       }
+}
+
 /**
  * Parse a switch statement.
  */
@@ -8462,6 +8488,7 @@ static statement_t *parse_switch(void)
        PUSH_PARENT(statement);
 
        expect('(');
+       add_anchor_token(')');
        expression_t *const expr = parse_expression();
        type_t       *      type = skip_typeref(expr->base.type);
        if (is_type_integer(type)) {
@@ -8473,6 +8500,7 @@ static statement_t *parse_switch(void)
        }
        statement->switchs.expression = create_implicit_cast(expr, type);
        expect(')');
+       rem_anchor_token(')');
 
        switch_statement_t *rem = current_switch;
        current_switch          = &statement->switchs;
@@ -8480,9 +8508,11 @@ static statement_t *parse_switch(void)
        current_switch          = rem;
 
        if (warning.switch_default &&
-          find_default_label(&statement->switchs) == NULL) {
+           statement->switchs.default_label == NULL) {
                warningf(&statement->base.source_position, "switch has no default case");
        }
+       if (warning.switch_enum)
+               check_enum_cases(&statement->switchs);
 
        POP_PARENT;
        return statement;

diff --git a/warning.c b/warning.c
index ad9f1aa..7329731 100644 (file)
--- a/warning.c
+++ b/warning.c
@@ -47,6 +47,7 @@ warning_t warning = {
        .sign_compare                  = false,
        .strict_prototypes             = true,
        .switch_default                = false,
+       .switch_enum                   = false,
        .unknown_pragmas               = true,
        .unreachable_code              = false,
        .unused_function               = false,
@@ -94,6 +95,7 @@ void set_warning_opt(const char *const opt)
                SET(unused_parameter);
                SET(unused_value);
                SET(unused_variable);
+               SET(switch_enum);
        }
        OPT("attribute",                     attribute);
        OPT("char-subscripts",               char_subscripts);
@@ -136,6 +138,7 @@ void set_warning_opt(const char *const opt)
        OPT("sign-compare",                  sign_compare);
        OPT("strict-prototypes",             strict_prototypes);
        OPT("switch-default",                switch_default);
+       OPT("switch-enum",                   switch_enum);
        OPT("unknown-pragmas",               unknown_pragmas);
        OPT("unreachable-code",              unreachable_code);
        OPTX("unused") {

diff --git a/warning.h b/warning.h
index 380161e..431dbd3 100644 (file)
--- a/warning.h
+++ b/warning.h
@@ -86,8 +86,8 @@ typedef struct warning_t {
 #endif
        bool strict_prototypes:1;             /**< Warn if a function declaration has an unspecified parameter list */
        bool switch_default:1;                /**< Warn whenever a 'switch' statement does not have a 'default' case */
-#if 0 // TODO
        bool switch_enum:1;                   /**< Warn about 'switch' statements with an enum as index type and missing case labels or case labels outside the enum range TODO has an alias -Wswitch? */
+#if 0 // TODO
        bool traditional:1;                   /**< Warn about certain constructs that behave differently in traditional and ISO C */
        bool undef:1;                         /**< Warn if an undefined identifier is evaluated in an '#if' directive */
        bool uninitialized:1;                 /**< Warn if an automatic variable is used without being initialized or if a variable may be clobbered by a 'setjmp' call. */