This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Prevent 'control reaches end of non-void function' warning for DO_WHILE


On 01/22/2012 03:38 AM, Tom de Vries wrote:

Sorry I didn't notice this patch until now; please CC me on C++ patches, or at least mention C++ in the subject line.

+      tree expr = NULL;
+      append_to_statement_list (*block, &expr);
+      *block = expr;

Rather than doing this dance here, I think it would be better to enhance append_to_statement_list to handle the case of the list argument being a non-list.


+ cp_walk_tree (&incr, cp_genericize_r, data, NULL);

+  if (incr && EXPR_P (incr))
+    SET_EXPR_LOCATION (incr, start_locus);

It might be better to set the location on incr before genericizing, so that the location can trickle down.


+         t = build1 (GOTO_EXPR, void_type_node, get_bc_label (bc_break));
+         SET_EXPR_LOCATION (t, start_locus);

Here you can use build1_loc instead of two separate statements.


-      /* If we use a LOOP_EXPR here, we have to feed the whole thing
-        back through the main gimplifier to lower it.  Given that we
-        have to gimplify the loop body NOW so that we can resolve
-        break/continue stmts, seems easier to just expand to gotos.  */

Since we're now lowering the loop at genericize time rather than gimplify, what's the rationale for not using LOOP_EXPR/EXIT_EXPR? We should still say something here. I suppose the rationale is that the C front end currently goes straight to gotos.


          if (cond != error_mark_node)
            {
-             gimplify_expr (&cond, &exit_seq, NULL, is_gimple_val, fb_rvalue);
-             stmt = gimple_build_cond (NE_EXPR, cond,
-                                       build_int_cst (TREE_TYPE (cond), 0),
-                                       gimple_label_label (top),
-                                       get_bc_label (bc_break));
-             gimple_seq_add_stmt (&exit_seq, stmt);
+             cond = build2 (NE_EXPR, boolean_type_node, cond,
+                            build_int_cst (TREE_TYPE (cond), 0));

I don't think we still need this extra comparison to 0, that seems like a gimple-specific thing.


   if (FOR_INIT_STMT (stmt))
+    append_to_statement_list (FOR_INIT_STMT (stmt), &expr);

+  genericize_cp_loop (&loop, EXPR_LOCATION (stmt), FOR_COND (stmt),
+                     FOR_BODY (stmt), FOR_EXPR (stmt), 1, walk_subtrees, data);

The call to genericize_cp_loop will clear *walk_subtrees, which means we don't genericize the FOR_INIT_STMT.


+ tree jump = build_and_jump (&label);

Again, let's use build1_loc.


+  *stmt_p = build_and_jump (&label);
+  SET_EXPR_LOCATION (*stmt_p, location);

And here.


+ stmt = make_node (OMP_FOR);

Why make a new OMP_FOR rather than repurpose the one we already have? We've already modified its operands.


Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]