Implement -Wparentheses.

[r24375]

diff --git a/ast.c b/ast.c
index 38b9980..498393c 100644 (file)
--- a/ast.c
+++ b/ast.c
@@ -758,10 +758,13 @@ static void print_expression_prec(const expression_t *expression, unsigned top_p
        if (expression->kind == EXPR_UNARY_CAST_IMPLICIT && !print_implicit_casts) {
                expression = expression->unary.value;
        }
-       unsigned prec = get_expression_precedence(expression->base.kind);
-       if (print_parenthesis && top_prec != PREC_BOTTOM)
-               top_prec = PREC_TOP;
-       if (top_prec > prec)
+
+       bool parenthesized =
+               expression->base.parenthesized                 ||
+               (print_parenthesis && top_prec != PREC_BOTTOM) ||
+               top_prec > get_expression_precedence(expression->base.kind);
+
+       if (parenthesized)
                fputc('(', out);
        switch (expression->kind) {
        case EXPR_UNKNOWN:
@@ -848,7 +851,7 @@ static void print_expression_prec(const expression_t *expression, unsigned top_p
                fprintf(out, "some expression of type %d", (int)expression->kind);
                break;
        }
-       if (top_prec > prec)
+       if (parenthesized)
                fputc(')', out);
 }
 

diff --git a/ast_t.h b/ast_t.h
index c0955ab..ab3e6ee 100644 (file)
--- a/ast_t.h
+++ b/ast_t.h
@@ -230,6 +230,7 @@ struct expression_base_t {
        expression_kind_t   kind;
        type_t             *type;
        source_position_t   source_position;
+       bool                parenthesized;
 #ifndef NDEBUG
        bool                transformed;
 #endif

diff --git a/cparser.1 b/cparser.1
index 1421688..c16c232 100644 (file)
--- a/cparser.1
+++ b/cparser.1
@@ -1,5 +1,5 @@
 .\" Please adjust this date whenever revising the manpage.
-.Dd November 29, 2008
+.Dd December 7, 2008
 .Dt CPARSER 1
 .Sh NAME
 .Nm cparser
@@ -114,6 +114,7 @@ In particular these are
 .Fl Winit-self ,
 .Fl Wmain ,
 .Fl Wnonnull ,
+.Fl Wparentheses ,
 .Fl Wpointer-arith ,
 .Fl Wredundant-decls ,
 .Fl Wreturn-type ,
@@ -178,6 +179,12 @@ Warn if a global function is defined without a previous prototype declaration.
 Warn if a multicharacter constant ('FOOF') is used.
 .It Fl Wnested-externs
 Warn if an 'extern' declaration is encountered within a function.
+.It Fl Wparentheses
+Warn if parentheses are omitted in certain contexts.
+Warn if an assignment is used as condition, e.g. if\ (x\ =\ 23).
+Warn if && without parentheses is used within ||, e.g. if\ (x\ ||\ y\ &&\ z).
+Warn if it there may be confusion which 'if'-statement an 'else'-branch belongs to, e.g. if\ (x)\ if\ (y)\ {}\ else\ {}.
+Warn if cascaded comparisons appear which do not have their mathematical meaning, e.g. if\ (23\ <=\ x\ <\ 42).
 .It Fl Wredundant-decls
 Warn about redundant declarations, i.e. multiple declarations of the same object or static forward declarations which have no use before their definition.
 .It Fl Wunreachable-code
@@ -190,9 +197,9 @@ Activate
 .Fl Wunused-value ,
 .Fl Wunused-variable .
 .It Fl Wunused-parameter
-Warn when a parameter is never used or only ever read to calculate its own new value, e.g. x = x + 1.
+Warn when a parameter is never used or only ever read to calculate its own new value, e.g. x\ =\ x\ +\ 1.
 .It Fl Wunused-variable
-Warn when a variable is never used or only ever read to calculate its own new value, e.g. x = x + 1.
+Warn when a variable is never used or only ever read to calculate its own new value, e.g. x\ =\ x\ +\ 1.
 .It Fl w
 Suppress all warnings.
 .It Fl I Ar dir

diff --git a/parser.c b/parser.c
index bfb4d3d..b8c07cc 100644 (file)
--- a/parser.c
+++ b/parser.c
@@ -7227,6 +7227,7 @@ static expression_t *parse_parenthesized_expression(void)
 
        add_anchor_token(')');
        expression_t *result = parse_expression();
+       result->base.parenthesized = true;
        rem_anchor_token(')');
        expect(')', end_error);
 
@@ -8188,12 +8189,25 @@ static void warn_reference_address_as_bool(expression_t const* expr)
        }
 }
 
+static void warn_assignment_in_condition(const expression_t *const expr)
+{
+       if (!warning.parentheses)
+               return;
+       if (expr->base.kind != EXPR_BINARY_ASSIGN)
+               return;
+       if (expr->base.parenthesized)
+               return;
+       warningf(&expr->base.source_position,
+                       "suggest parentheses around assignment used as truth value");
+}
+
 static void semantic_condition(expression_t const *const expr,
                                char const *const context)
 {
        type_t *const type = skip_typeref(expr->base.type);
        if (is_type_scalar(type)) {
                warn_reference_address_as_bool(expr);
+               warn_assignment_in_condition(expr);
        } else if (is_type_valid(type)) {
                errorf(&expr->base.source_position,
                                "%s must have scalar type", context);
@@ -8929,6 +8943,25 @@ static void warn_string_literal_address(expression_t const* expr)
        }
 }
 
+static void warn_comparison_in_comparison(const expression_t *const expr)
+{
+       if (expr->base.parenthesized)
+               return;
+       switch (expr->base.kind) {
+               case EXPR_BINARY_LESS:
+               case EXPR_BINARY_GREATER:
+               case EXPR_BINARY_LESSEQUAL:
+               case EXPR_BINARY_GREATEREQUAL:
+               case EXPR_BINARY_NOTEQUAL:
+               case EXPR_BINARY_EQUAL:
+                       warningf(&expr->base.source_position,
+                                       "comparisons like 'x <= y < z' do not have their mathematical meaning");
+                       break;
+               default:
+                       break;
+       }
+}
+
 /**
  * Check the semantics of comparison expressions.
  *
@@ -8958,6 +8991,11 @@ static void semantic_comparison(binary_expression_t *expression)
                }
        }
 
+       if (warning.parentheses) {
+               warn_comparison_in_comparison(left);
+               warn_comparison_in_comparison(right);
+       }
+
        type_t *orig_type_left  = left->base.type;
        type_t *orig_type_right = right->base.type;
        type_t *type_left       = skip_typeref(orig_type_left);
@@ -9148,6 +9186,16 @@ static void semantic_arithmetic_addsubb_assign(binary_expression_t *expression)
        }
 }
 
+static void warn_logical_and_within_or(const expression_t *const expr)
+{
+       if (expr->base.kind != EXPR_BINARY_LOGICAL_AND)
+               return;
+       if (expr->base.parenthesized)
+               return;
+       warningf(&expr->base.source_position,
+                       "suggest parentheses around && within ||");
+}
+
 /**
  * Check the semantic restrictions of a logical expression.
  */
@@ -9157,6 +9205,11 @@ static void semantic_logical_op(binary_expression_t *expression)
         * §6.5.14:2  Each of the operands shall have scalar type. */
        semantic_condition(expression->left,   "left operand of logical operator");
        semantic_condition(expression->right, "right operand of logical operator");
+       if (expression->base.kind == EXPR_BINARY_LOGICAL_OR &&
+                       warning.parentheses) {
+               warn_logical_and_within_or(expression->left);
+               warn_logical_and_within_or(expression->right);
+       }
        expression->base.type = c_mode & _CXX ? type_bool : type_int;
 }
 
@@ -9939,12 +9992,18 @@ end_error:
        rem_anchor_token('{');
 
        add_anchor_token(T_else);
-       statement->ifs.true_statement = parse_statement();
+       statement_t *const true_stmt = parse_statement();
+       statement->ifs.true_statement = true_stmt;
        rem_anchor_token(T_else);
 
        if (token.type == T_else) {
                next_token();
                statement->ifs.false_statement = parse_statement();
+       } else if (warning.parentheses &&
+                       true_stmt->kind == STATEMENT_IF &&
+                       true_stmt->ifs.false_statement != NULL) {
+               warningf(&true_stmt->base.source_position,
+                               "suggest explicit braces to avoid ambiguous 'else'");
        }
 
        POP_PARENT;

diff --git a/warning.c b/warning.c
index 8de3339..2c4ce12 100644 (file)
--- a/warning.c
+++ b/warning.c
@@ -53,6 +53,7 @@ warning_t warning = {
        .old_style_definition                = false,
        .packed                              = false,
        .padded                              = false,
+       .parentheses                         = false,
        .pointer_arith                       = true,
        .redundant_decls                     = true,
        .return_type                         = true,
@@ -104,6 +105,7 @@ void set_warning_opt(const char *const opt)
                SET(init_self);
                SET(main);
                SET(nonnull);
+               SET(parentheses);
                SET(pointer_arith);
                SET(redundant_decls);
                SET(return_type);
@@ -166,6 +168,7 @@ void set_warning_opt(const char *const opt)
        OPT("old-style-definition",                old_style_definition);
        OPT("packed",                              packed);
        OPT("padded",                              padded);
+       OPT("parentheses",                         parentheses);
        OPT("pointer-arith",                       pointer_arith);
        OPT("redundant-decls",                     redundant_decls);
        OPT("return-type",                         return_type);

diff --git a/warning.h b/warning.h
index b53d2b3..fdc859d 100644 (file)
--- a/warning.h
+++ b/warning.h
@@ -77,9 +77,7 @@ typedef struct warning_t {
        bool old_style_definition:1;                /**< Warn if an old-style function definition is used. */
        bool packed:1;                              /**< Warn if a structure is given the packed attribute, but the packed attribute has no effect on the layout or size of the structure */
        bool padded:1;                              /**< Warn if padding is included in a structure, either to align an element of the structure or to align the whole structure */
-#if 0 // TODO
        bool parentheses:1;                         /**< Warn if parentheses are omitted in certain contexts (assignment where truth value is expected, if-else-braces) */
-#endif
        bool pointer_arith:1;                       /**< Warn about anything that depends on the "size of" a function type or of 'void' */
 #if 0 // TODO
        bool pointer_to_int_cast:1;                 /**< Warn if cast from pointer to integer of different size. */