Changeset 26617 in webkit for trunk/JavaScriptCore

Timestamp:

Oct 15, 2007, 1:36:57 PM (18 years ago)

Author:

ggaren

Message:

JavaScriptCore:

Reviewed by Maciej Stachowiak.

Fixed ​http://bugs.webkit.org/show_bug.cgi?id=15490
Iteration statements sometimes incorrectly evaluate to the empty value
(KDE r670547).

[ Broken off from ​http://bugs.webkit.org/show_bug.cgi?id=14868 ]

This patch is a merge of KDE r670547, with substantial modification
for performance.

It fixes do-while statements to evaluate to a value. (They used
to evaluate to the empty value in all cases.)

It also fixes SourceElementsNode to maintain the value of abnormal
completions like "break" and "continue."

It also re-works the main execution loop in SourceElementsNode so that
it (1) makes a little more sense and (2) avoids unnecessary work. This
is a .28% speedup on command-line JS iBench.

• kjs/nodes.cpp: (DoWhileNode::execute): (SourceElementsNode::execute):

LayoutTests:

Reviewed by Maciej Stachowiak.

Layout tests for ​http://bugs.webkit.org/show_bug.cgi?id=15490
Iteration statements sometimes incorrectly evaluate to the empty value
(KDE r670547)

• fast/js/do-while-expression-value-expected.txt: Added.
• fast/js/do-while-expression-value.html: Added.
• fast/js/while-expression-value-expected.txt: Added.
• fast/js/while-expression-value.html: Added.

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• kjs/completion.h (modified) (1 diff)
• kjs/nodes.cpp (modified) (4 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2007-10-12  Geoffrey Garen  <[email protected]>
+
+        Reviewed by Maciej Stachowiak.
+        
+        Fixed http://bugs.webkit.org/show_bug.cgi?id=15490
+        Iteration statements sometimes incorrectly evaluate to the empty value 
+        (KDE r670547). 
+        
+        [ Broken off from http://bugs.webkit.org/show_bug.cgi?id=14868 ]
+        
+        This patch is a merge of KDE r670547, with substantial modification 
+        for performance.
+        
+        It fixes do-while statements to evaluate to a value. (They used
+        to evaluate to the empty value in all cases.) 
+
+        It also fixes SourceElementsNode to maintain the value of abnormal 
+        completions like "break" and "continue."
+        
+        It also re-works the main execution loop in SourceElementsNode so that
+        it (1) makes a little more sense and (2) avoids unnecessary work. This 
+        is a .28% speedup on command-line JS iBench.
+
+        * kjs/nodes.cpp:
+        (DoWhileNode::execute):
+        (SourceElementsNode::execute):
+
 2007-10-15  Simon Hausmann  <[email protected]>
 

trunk/JavaScriptCore/kjs/completion.h

@@ -53 +53 @@
     ComplType complType() const { return comp; }
     JSValue *value() const { return val; }
+    void setValue(JSValue* v) { val = v; }
     Identifier target() const { return tar; }
     bool isValueCompletion() const { return !!val; }

trunk/JavaScriptCore/kjs/nodes.cpp

@@ -1808 +1808 @@
   JSValue *bv;
   Completion c;
+  JSValue* value = 0;
 
   do {
@@ -1820 +1821 @@
         return Completion(Interrupted);
 
+    if (c.isValueCompletion())
+        value = c.value();
+
     if (!((c.complType() == Continue) && ls.contains(c.target()))) {
       if ((c.complType() == Break) && ls.contains(c.target()))
-        return Completion(Normal, 0);
+        return Completion(Normal, value);
       if (c.complType() != Normal)
         return c;
@@ -1830 +1834 @@
   } while (bv->toBoolean(exec));
 
-  return Completion(Normal, 0);
+  return Completion(Normal, value);
 }
 
@@ -2560 +2564 @@
 {
   KJS_CHECKEXCEPTION
-
-  Completion c1 = node->execute(exec);
-  KJS_CHECKEXCEPTION;
-  if (c1.complType() != Normal)
-    return c1;
-  
-  for (SourceElementsNode *n = next.get(); n; n = n->next.get()) {
-    Completion c2 = n->node->execute(exec);
-    if (c2.complType() != Normal)
-      return c2;
-    // The spec says to return c2 here, but it seems that mozilla returns c1 if
-    // c2 doesn't have a value
-    if (c2.value())
-      c1 = c2;
-  }
-  
-  return c1;
+  JSValue* v = 0;
+  SourceElementsNode* n = this;
+  while (1) {
+    Completion c = n->node->execute(exec);
+
+    if (JSValue* v2 = c.value())
+      v = v2;
+    c.setValue(v);
+
+    if (c.complType() != Normal)
+        return c;
+
+    n = n->next.get();
+    if (!n)
+        return c;
+  }
 }