Bug 18192 - Squirrelfish needs support for break and continue
Summary: Squirrelfish needs support for break and continue
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Oliver Hunt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-28 14:40 PDT by Oliver Hunt
Modified: 2008-03-28 15:40 PDT (History)
4 users (show)

See Also:


Attachments
Implementate support for continue and goto (9.67 KB, patch)
2008-03-28 14:47 PDT, Oliver Hunt
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver Hunt 2008-03-28 14:40:05 PDT
Task tracking bug
Comment 1 Oliver Hunt 2008-03-28 14:47:12 PDT
Created attachment 20165 [details]
Implementate support for continue and goto
Comment 2 Geoffrey Garen 2008-03-28 15:00:53 PDT
+        Vector<LoopContext> m_loopsContextStack;

How about calling this "m_loopContextStack"?

+CodeGenerator::LoopContext* CodeGenerator::scopeForIdentifier(const Identifier& label)

"Scope" is very specifically tied to variable resolution in JavaScript. How about naming this function "loopContextForIdentifier"?

+    LoopContext* scope = scopeForIdentifier(label);

And calling this variable "loopContext"?

+LabelID* CodeGenerator::getLabelForContinue(const Identifier& label)
+{
+    LoopContext* scope = scopeForIdentifier(label);
+    return scope ? scope->continueTarget : 0;
+}
+
+LabelID* CodeGenerator::getLabelForBreak(const Identifier& label)

Our style is to omit the "get" from these function names.

Looks good. r=me
Comment 3 Geoffrey Garen 2008-03-28 15:01:34 PDT
I think you can change this:

+    LoopContext scope = { labels, continueTarget, breakTarget };
+    m_loopsContextStack.append(scope);

to this:

m_loopsContextStack.append({ labels, continueTarget, breakTarget });
Comment 4 Oliver Hunt 2008-03-28 15:38:37 PDT
Comment on attachment 20165 [details]
Implementate support for continue and goto

reviewed by Geoff
Comment 5 Oliver Hunt 2008-03-28 15:40:42 PDT
Landed r31411