[cfe-dev] Patch: Implement break and continue

[Chris Lattner]

On Jul 15, 2007, at 9:05 PM, Anders Carlsson wrote:
> Hello,
> here's a patch that implements break and continue in for, while and  
> do loops.
>
> Since this is my first encounter ever with LLVM and cfe, the patch  
> could very well be completely wrong :) I'd love any feedback it  
> though.

I have to say that this is *not* how I envisioned implementing this:  
it's actually a much better approach.

Overall, I really like the patch.  Some suggestions:

1. Instead of having separate break/continue stacks, I'd suggest  
merging them and using a SmallVector<std::pair> (or a custom struct  
instead of a pair if you prefer).  For switch stmts (when we support  
them), we'd just use null as the continue point.


2. Two minor style issues:

+void CodeGenFunction::EmitBreakStmt()
+{

Please put the { on the same line as the function.

+    assert(!BreakStack.empty());

Please include some sort of message in the assert so that we know  
"why" that condition should be true.  This doesn't need to be  
something particularly deep, just something like this should be  
sufficient:

+    assert(!BreakStack.empty() && "break stmt not in a loop or  
switch?");


3. Some extremely minor typographic stuff:

Minor typo: "foo":

+  // If the foor loop doesn't have an increment we can just use the

Please end sentences with a period: :)

+  // If the foor loop doesn't have an increment we can just use the
+  // condition as the continue block


Otherwise the patch looks great.  Please update and resubmit,  (not  
bad for a first encounter ;-)

-Chris