Some side-effect propagation for better warnings, so that comma-operators with assignments in them don't cause the -Weffectless-statement warning, and fixing ternary precedence for fte operator list

ast.c

@@ -35,6 +35,7 @@
     ast_node_init((ast_node*)self, ctx, TYPE_##T);                  \
     ( (ast_node*)self )->node.destroy = (ast_node_delete*)destroyfn
 
+
 /* error handling */
 static void asterror(lex_ctx ctx, const char *msg, ...)
 {
@@ -59,8 +60,17 @@ static void ast_node_init(ast_node *self, lex_ctx ctx, int nodetype)
     self->node.destroy = &_ast_node_destroy;
     self->node.keep    = false;
     self->node.nodetype = nodetype;
+    self->node.side_effects = false;
 }
 
+/* weight and side effects */
+static void _ast_propagate_effects(ast_node *self, ast_node *other)
+{
+    if (ast_side_effects(other))
+        ast_side_effects(self) = true;
+}
+#define ast_propagate_effects(s,o) _ast_propagate_effects(((ast_node*)(s)), ((ast_node*)(o)))
+
 /* General expression initialization */
 static void ast_expression_init(ast_expression *self,
                                 ast_expression_codegen *codegen)
@@ -385,6 +395,9 @@ ast_binary* ast_binary_new(lex_ctx ctx, int op,
     self->left = left;
     self->right = right;
 
+    ast_propagate_effects(self, left);
+    ast_propagate_effects(self, right);
+
     if (op >= INSTR_EQ_F && op <= INSTR_GT)
         self->expression.vtype = TYPE_FLOAT;
     else if (op == INSTR_AND || op == INSTR_OR ||
@@ -414,6 +427,8 @@ ast_binstore* ast_binstore_new(lex_ctx ctx, int storop, int op,
     ast_instantiate(ast_binstore, ctx, ast_binstore_delete);
     ast_expression_init((ast_expression*)self, (ast_expression_codegen*)&ast_binstore_codegen);
 
+    ast_side_effects(self) = true;
+
     self->opstore = storop;
     self->opbin   = op;
     self->dest    = left;
@@ -450,6 +465,8 @@ ast_unary* ast_unary_new(lex_ctx ctx, int op,
     self->op = op;
     self->operand = expr;
 
+    ast_propagate_effects(self, expr);
+
     if (op >= INSTR_NOT_F && op <= INSTR_NOT_FNC) {
         self->expression.vtype = TYPE_FLOAT;
     } else
@@ -472,6 +489,9 @@ ast_return* ast_return_new(lex_ctx ctx, ast_expression *expr)
 
     self->operand = expr;
 
+    if (expr)
+        ast_propagate_effects(self, expr);
+
     return self;
 }
 
@@ -506,6 +526,8 @@ ast_entfield* ast_entfield_new_force(lex_ctx ctx, ast_expression *entity, ast_ex
 
     self->entity = entity;
     self->field  = field;
+    ast_propagate_effects(self, entity);
+    ast_propagate_effects(self, field);
 
     if (!ast_type_adopt(self, outtype)) {
         ast_entfield_delete(self);
@@ -550,6 +572,8 @@ ast_member* ast_member_new(lex_ctx ctx, ast_expression *owner, unsigned int fiel
     }
 
     self->owner = owner;
+    ast_propagate_effects(self, owner);
+
     self->field = field;
     if (name)
         self->name = util_strdup(name);
@@ -589,6 +613,8 @@ ast_array_index* ast_array_index_new(lex_ctx ctx, ast_expression *array, ast_exp
 
     self->array = array;
     self->index = index;
+    ast_propagate_effects(self, array);
+    ast_propagate_effects(self, index);
 
     if (!ast_type_adopt(self, outtype)) {
         ast_array_index_delete(self);
@@ -628,6 +654,11 @@ ast_ifthen* ast_ifthen_new(lex_ctx ctx, ast_expression *cond, ast_expression *on
     self->cond     = cond;
     self->on_true  = ontrue;
     self->on_false = onfalse;
+    ast_propagate_effects(self, cond);
+    if (ontrue)
+        ast_propagate_effects(self, ontrue);
+    if (onfalse)
+        ast_propagate_effects(self, onfalse);
 
     return self;
 }
@@ -656,6 +687,9 @@ ast_ternary* ast_ternary_new(lex_ctx ctx, ast_expression *cond, ast_expression *
     self->cond     = cond;
     self->on_true  = ontrue;
     self->on_false = onfalse;
+    ast_propagate_effects(self, cond);
+    ast_propagate_effects(self, ontrue);
+    ast_propagate_effects(self, onfalse);
 
     if (!ast_type_adopt(self, ontrue)) {
         ast_ternary_delete(self);
@@ -690,6 +724,17 @@ ast_loop* ast_loop_new(lex_ctx ctx,
     self->increment = increment;
     self->body      = body;
 
+    if (initexpr)
+        ast_propagate_effects(self, initexpr);
+    if (precond)
+        ast_propagate_effects(self, precond);
+    if (postcond)
+        ast_propagate_effects(self, postcond);
+    if (increment)
+        ast_propagate_effects(self, increment);
+    if (body)
+        ast_propagate_effects(self, body);
+
     return self;
 }
 
@@ -733,6 +778,8 @@ ast_switch* ast_switch_new(lex_ctx ctx, ast_expression *op)
     self->operand = op;
     self->cases   = NULL;
 
+    ast_propagate_effects(self, op);
+
     return self;
 }
 
@@ -758,6 +805,8 @@ ast_call* ast_call_new(lex_ctx ctx,
     ast_instantiate(ast_call, ctx, ast_call_delete);
     ast_expression_init((ast_expression*)self, (ast_expression_codegen*)&ast_call_codegen);
 
+    ast_side_effects(self) = true;
+
     self->params = NULL;
     self->func   = funcexpr;
 
@@ -812,6 +861,8 @@ ast_store* ast_store_new(lex_ctx ctx, int op,
     ast_instantiate(ast_store, ctx, ast_store_delete);
     ast_expression_init((ast_expression*)self, (ast_expression_codegen*)&ast_store_codegen);
 
+    ast_side_effects(self) = true;
+
     self->op = op;
     self->dest = dest;
     self->source = source;
@@ -851,6 +902,12 @@ ast_block* ast_block_new(lex_ctx ctx)
     return self;
 }
 
+void ast_block_add_expr(ast_block *self, ast_expression *e)
+{
+    ast_propagate_effects(self, e);
+    vec_push(self->exprs, e);
+}
+
 void ast_block_collect(ast_block *self, ast_expression *expr)
 {
     vec_push(self->collect, expr);

ast.h

@@ -73,6 +73,7 @@ enum {
 
 #define ast_istype(x, t) ( ((ast_node_common*)x)->nodetype == (TYPE_##t) )
 #define ast_ctx(node) (((ast_node_common*)(node))->context)
+#define ast_side_effects(node) (((ast_node_common*)(node))->side_effects)
 
 /* Node interface with common components
  */
@@ -87,6 +88,7 @@ typedef struct
      * prevents its dtor from destroying this node as well.
      */
     bool             keep;
+    bool             side_effects;
 } ast_node_common;
 
 #define ast_delete(x) ( ( (ast_node*)(x) ) -> node.destroy )((ast_node*)(x))
@@ -526,6 +528,8 @@ bool ast_block_set_type(ast_block*, ast_expression *from);
 bool ast_block_codegen(ast_block*, ast_function*, bool lvalue, ir_value**);
 void ast_block_collect(ast_block*, ast_expression*);
 
+void ast_block_add_expr(ast_block*, ast_expression*);
+
 /* Function
  *
  * Contains a list of blocks... at least in theory.

lexer.h

@@ -252,6 +252,9 @@ static const oper_info fte_operators[] = {
     { "==",  2, opid2('=','='),     ASSOC_LEFT,  10,  0 },
     { "!=",  2, opid2('!','='),     ASSOC_LEFT,  10,  0 },
 
+    { "?",   3, opid2('?',':'),     ASSOC_RIGHT, 9,  0 },
+    { ":",   3, opid2(':','?'),     ASSOC_RIGHT, 9,  0 },
+
     { "=",   2, opid1('='),         ASSOC_RIGHT, 8,  0 },
     { "+=",  2, opid2('+','='),     ASSOC_RIGHT, 8,  0 },
     { "-=",  2, opid2('-','='),     ASSOC_RIGHT, 8,  0 },
@@ -265,10 +268,7 @@ static const oper_info fte_operators[] = {
     { "&&",  2, opid2('&','&'),     ASSOC_LEFT,  5,  0 },
     { "||",  2, opid2('|','|'),     ASSOC_LEFT,  5,  0 },
 
-    { ",",   2, opid1(','),         ASSOC_LEFT,  2,  0 },
+    { ",",   2, opid1(','),         ASSOC_LEFT,  2,  0 }
-
-    { "?",   3, opid2('?',':'),     ASSOC_RIGHT, 1,  0 },
-    { ":",   3, opid2(':','?'),     ASSOC_RIGHT, 1,  0 }
 };
 static const size_t fte_operator_count = (sizeof(fte_operators) / sizeof(fte_operators[0]));
 

parser.c

@@ -575,11 +575,11 @@ static bool parser_sy_pop(parser_t *parser, shunt *sy)
 
         case opid1(','):
             if (blocks[0]) {
-                vec_push(blocks[0]->exprs, exprs[1]);
+                ast_block_add_expr(blocks[0], exprs[1]);
             } else {
                 blocks[0] = ast_block_new(ctx);
-                vec_push(blocks[0]->exprs, exprs[0]);
+                ast_block_add_expr(blocks[0], exprs[0]);
-                vec_push(blocks[0]->exprs, exprs[1]);
+                ast_block_add_expr(blocks[0], exprs[1]);
             }
             if (!ast_block_set_type(blocks[0], exprs[1]))
                 return false;
@@ -1961,10 +1961,7 @@ static bool parse_for(parser_t *parser, ast_block *block, ast_expression **out)
         increment = parse_expression_leave(parser, false);
         if (!increment)
             goto onerr;
-        if (!ast_istype(increment, ast_store) &&
+        if (!ast_side_effects(increment)) {
-            !ast_istype(increment, ast_call) &&
-            !ast_istype(increment, ast_binstore))
-        {
             if (genwarning(ast_ctx(increment), WARN_EFFECTLESS_STATEMENT, "statement has no effect"))
                 goto onerr;
         }
@@ -2187,7 +2184,7 @@ static bool parse_switch(parser_t *parser, ast_block *block, ast_expression **ou
             }
             if (!expr)
                 continue;
-            vec_push(caseblock->exprs, expr);
+            ast_block_add_expr(caseblock, expr);
         }
     }
 
@@ -2316,10 +2313,7 @@ static bool parse_statement(parser_t *parser, ast_block *block, ast_expression *
         if (!exp)
             return false;
         *out = exp;
-        if (!ast_istype(exp, ast_store) &&
+        if (!ast_side_effects(exp)) {
-            !ast_istype(exp, ast_call) &&
-            !ast_istype(exp, ast_binstore))
-        {
             if (genwarning(ast_ctx(exp), WARN_EFFECTLESS_STATEMENT, "statement has no effect"))
                 return false;
         }
@@ -2351,7 +2345,7 @@ static bool parse_block_into(parser_t *parser, ast_block *block, bool warnreturn
         }
         if (!expr)
             continue;
-        vec_push(block->exprs, expr);
+        ast_block_add_expr(block, expr);
     }
 
     if (parser->tok != '}') {
@@ -2623,9 +2617,9 @@ static bool parse_function_body(parser_t *parser, ast_value *var)
                 if (store_think)     ast_delete(store_think);
                 retval = false;
             }
-            vec_push(block->exprs, (ast_expression*)store_frame);
+            ast_block_add_expr(block, (ast_expression*)store_frame);
-            vec_push(block->exprs, (ast_expression*)store_nextthink);
+            ast_block_add_expr(block, (ast_expression*)store_nextthink);
-            vec_push(block->exprs, (ast_expression*)store_think);
+            ast_block_add_expr(block, (ast_expression*)store_think);
         }
 
         if (!retval) {
@@ -2774,7 +2768,7 @@ static ast_expression *array_setter_node(parser_t *parser, ast_value *array, ast
             return NULL;
         }
 
-        vec_push(block->exprs, (ast_expression*)st);
+        ast_block_add_expr(block, (ast_expression*)st);
 
         ret = ast_return_new(ctx, NULL);
         if (!ret) {
@@ -2782,7 +2776,7 @@ static ast_expression *array_setter_node(parser_t *parser, ast_value *array, ast
             return NULL;
         }
 
-        vec_push(block->exprs, (ast_expression*)ret);
+        ast_block_add_expr(block, (ast_expression*)ret);
 
         return (ast_expression*)block;
     } else {
@@ -2843,7 +2837,7 @@ static ast_expression *array_field_setter_node(
             return NULL;
         }
 
-        vec_push(block->exprs, (ast_expression*)st);
+        ast_block_add_expr(block, (ast_expression*)st);
 
         ret = ast_return_new(ctx, NULL);
         if (!ret) {
@@ -2851,7 +2845,7 @@ static ast_expression *array_field_setter_node(
             return NULL;
         }
 
-        vec_push(block->exprs, (ast_expression*)ret);
+        ast_block_add_expr(block, (ast_expression*)ret);
 
         return (ast_expression*)block;
     } else {
@@ -2963,7 +2957,7 @@ static bool parser_create_array_setter(parser_t *parser, ast_value *array, const
         goto cleanup;
     }
 
-    vec_push(func->blocks[0]->exprs, root);
+    ast_block_add_expr(func->blocks[0], root);
     array->setter = fval;
     return true;
 cleanup:
@@ -3012,7 +3006,7 @@ static bool parser_create_array_field_setter(parser_t *parser, ast_value *array,
         goto cleanup;
     }
 
-    vec_push(func->blocks[0]->exprs, root);
+    ast_block_add_expr(func->blocks[0], root);
     array->setter = fval;
     return true;
 cleanup:
@@ -3059,7 +3053,7 @@ static bool parser_create_array_getter(parser_t *parser, ast_value *array, const
         goto cleanup;
     }
 
-    vec_push(func->blocks[0]->exprs, root);
+    ast_block_add_expr(func->blocks[0], root);
     array->getter = fval;
     return true;
 cleanup:
@@ -3848,7 +3842,7 @@ skipvar:
                 else {
                     if (vec_size(sy.out) != 1 && vec_size(sy.ops) != 0)
                         parseerror(parser, "internal error: leaked operands");
-                    vec_push(localblock->exprs, (ast_expression*)sy.out[0].out);
+                    ast_block_add_expr(localblock, (ast_expression*)sy.out[0].out);
                 }
                 vec_free(sy.out);
                 vec_free(sy.ops);