[PATCH] Fix memory leaks in tree-ssa-uninit.c

Bernd Schmidt bschmidt@redhat.com
Thu Nov 19 13:58:00 GMT 2015


On 11/19/2015 11:16 AM, Martin Liška wrote:
> You are right, however as the original coding style was really broken,
> it was much easier
> to use the tool and clean-up fall-out.
>
> Waiting for thoughts related to v2.

Better, but still some oddities. I hope you won't get mad at me if I 
suggest doing this in stages? A first patch could just deal with 
non-reformatting whitespace changes, such as removing trailing 
whitespace, and converting leading spaces to tabs - that would be 
mechanical, and reduce the size of the rest of the patch (it seems emacs 
has an appropriate command, M-x whitespace-cleanup). Such a change is 
preapproved.

A few things I noticed:

> -   flag_1 = phi <0, 1>                  // (1)
> +   flag_1 = phi <0, 1>		  // (1)

Check whether the // (1) is still lined up with the // (2) and // (3) 
parts. In general I'm not sure we should to what extent we should be 
reformatting comments in this patch. Breaking long lines and ensuring 
two spaces after a period seems fine.

> +   Checking recursively into (1), the compiler can find out that only some_val
> +   (which is defined) can flow into (3) which is OK.
>
>  */

Could take the opportunity to move the */ onto the end of the line.

> +	  if (is_gimple_call (cond_stmt) && EDGE_COUNT (e->src->succs) >= 2)
> +	    {
> +	      /* Ignore EH edge.  Can add assertion
> +		 on the other edge's flag.  */
> +	      continue;
> +	    }

Could also take the opportunity to remove excess braces and parentheses. 
Not a requirement and probably a distraction at this point.

> -  return (pred.cond_code == NE_EXPR && !pred.invert)
> -          || (pred.cond_code == EQ_EXPR && pred.invert);
> +  return (pred.cond_code == NE_EXPR && !pred.invert)
> +	 || (pred.cond_code == EQ_EXPR && pred.invert);

This still isn't right.


Bernd



More information about the Gcc-patches mailing list