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] Fix memory leaks in tree-ssa-uninit.c


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


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