Title: [259810] trunk
Revision
259810
Author
drou...@apple.com
Date
2020-04-09 11:22:00 -0700 (Thu, 09 Apr 2020)

Log Message

Web Inspector: Debugger: debug hooks should also be emitted for the first sub-_expression_ in a comma _expression_
https://bugs.webkit.org/show_bug.cgi?id=210253

Reviewed by Joseph Pecoraro.

Source/_javascript_Core:

* bytecompiler/NodesCodegen.cpp:
(JSC::CommaNode::emitBytecode):
* parser/Parser.cpp:
(JSC::Parser<LexerType>::parseVariableDeclarationList):
(JSC::Parser<LexerType>::parseExpression):
We should emit debug hooks and record pause locations for the first sub-_expression_ in comma
expressions, as the comma _expression_ is not always standalone (e.g. `true && (a(), b())`).

* bytecompiler/BytecodeGenerator.h:
* bytecompiler/BytecodeGenerator.cpp:
(JSC::BytecodeGenerator::emitDebugHook):
Save the `JSTextPosition` and `DebugHookType` of the last debug hook, using them to prevent
any additional debug hooks from being emitted if they have the same `JSTextPosition` and
`DebugHookType`. This prevents the debugger from pausing twice at the beginning of an
_expression_ statement (e.g. `|a(), b();`).

Source/WebInspectorUI:

* UserInterface/Workers/Formatter/JSFormatter.js:
(JSFormatter.prototype._handleTokenAtNode):
(JSFormatter.prototype._isLikelyToHaveNewline): Deleted.
If an arrow function wraps it's body with `{` and `}`, always add newlines to make setting
breakpoints inside the function body easier.

LayoutTests:

* inspector/debugger/breakpoints/resources/dump-general.js:
* inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt:
* inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt:
* inspector/debugger/stepping/stepOver.html:
* inspector/debugger/stepping/stepOver-expected.txt:
* inspector/formatting/formatting-_javascript_.html:
* inspector/formatting/formatting-_javascript_-expected.txt:
* inspector/formatting/resources/_javascript_-tests/arrow-functions-expected.js:
* inspector/formatting/resources/_javascript_-tests/comma-expressions.js: Added.
* inspector/formatting/resources/_javascript_-tests/comma-expressions-expected.js: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (259809 => 259810)


--- trunk/LayoutTests/ChangeLog	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/ChangeLog	2020-04-09 18:22:00 UTC (rev 259810)
@@ -1,3 +1,21 @@
+2020-04-09  Devin Rousso  <drou...@apple.com>
+
+        Web Inspector: Debugger: debug hooks should also be emitted for the first sub-_expression_ in a comma _expression_
+        https://bugs.webkit.org/show_bug.cgi?id=210253
+
+        Reviewed by Joseph Pecoraro.
+
+        * inspector/debugger/breakpoints/resources/dump-general.js:
+        * inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt:
+        * inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt:
+        * inspector/debugger/stepping/stepOver.html:
+        * inspector/debugger/stepping/stepOver-expected.txt:
+        * inspector/formatting/formatting-_javascript_.html:
+        * inspector/formatting/formatting-_javascript_-expected.txt:
+        * inspector/formatting/resources/_javascript_-tests/arrow-functions-expected.js:
+        * inspector/formatting/resources/_javascript_-tests/comma-expressions.js: Added.
+        * inspector/formatting/resources/_javascript_-tests/comma-expressions-expected.js: Added.
+
 2020-04-08  Simon Fraser  <simon.fra...@apple.com>
 
         [Async overflow scroll] Horizontal scrolls can trigger unwanted back swipes

Modified: trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt (259809 => 259810)


--- trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt	2020-04-09 18:22:00 UTC (rev 259810)
@@ -1411,6 +1411,7 @@
  => 214    |b(),
     215    c();
     216    
+    217    true && (a(), b(), c());
 
 INSERTING AT: 214:1
 PAUSES AT: 215:0
@@ -1420,8 +1421,44 @@
  -> 214    b#(),
  => 215    |c();
     216    
+    217    true && (a(), b(), c());
+    218    
 
+INSERTING AT: 215:1
+PAUSES AT: 217:0
+    212    
+    213    a(),
+    214    b(),
+ -> 215    c#();
+    216    
+ => 217    |true && (a(), b(), c());
+    218    
 
+INSERTING AT: 217:1
+PAUSES AT: 217:9
+    214    b(),
+    215    c();
+    216    
+-=> 217    t#rue && (|a(), b(), c());
+    218    
+
+INSERTING AT: 217:10
+PAUSES AT: 217:14
+    214    b(),
+    215    c();
+    216    
+-=> 217    true && (a#(), |b(), c());
+    218    
+
+INSERTING AT: 217:15
+PAUSES AT: 217:19
+    214    b(),
+    215    c();
+    216    
+-=> 217    true && (a(), b#(), |c());
+    218    
+
+
 -- Running test case: Debugger.resolvedBreakpoint.dumpAllLocations.Functions
 
 INSERTING AT: 0:0

Modified: trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt (259809 => 259810)


--- trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt	2020-04-09 18:22:00 UTC (rev 259810)
@@ -3093,6 +3093,7 @@
 -=> 214    |b(),
     215    c();
     216    
+    217    true && (a(), b(), c());
 
 
 INSERTING AT: 215:0
@@ -3102,9 +3103,30 @@
     214    b(),
 -=> 215    |c();
     216    
+    217    true && (a(), b(), c());
+    218    
 
 
 INSERTING AT: 216:0
+PAUSES AT: 217:0
+    213    a(),
+    214    b(),
+    215    c();
+ -> 216    #
+ => 217    |true && (a(), b(), c());
+    218    
+
+
+INSERTING AT: 217:0
+PAUSES AT: 217:0
+    214    b(),
+    215    c();
+    216    
+-=> 217    |true && (a(), b(), c());
+    218    
+
+
+INSERTING AT: 218:0
 PRODUCES: Could not resolve breakpoint
 
 -- Running test case: Debugger.resolvedBreakpoint.dumpEachLine.Functions

Modified: trunk/LayoutTests/inspector/debugger/breakpoints/resources/dump-general.js (259809 => 259810)


--- trunk/LayoutTests/inspector/debugger/breakpoints/resources/dump-general.js	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/inspector/debugger/breakpoints/resources/dump-general.js	2020-04-09 18:22:00 UTC (rev 259810)
@@ -214,3 +214,5 @@
 a(),
 b(),
 c();
+
+true && (a(), b(), c());

Modified: trunk/LayoutTests/inspector/debugger/stepping/stepOver-expected.txt (259809 => 259810)


--- trunk/LayoutTests/inspector/debugger/stepping/stepOver-expected.txt	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/inspector/debugger/stepping/stepOver-expected.txt	2020-04-09 18:22:00 UTC (rev 259810)
@@ -1,7 +1,3 @@
-ALERT: function log 1
-ALERT: comma log 1
-ALERT: comma log 2
-ALERT: comma log 3
 Checking pause locations when stepping with "stepOver".
 
 
@@ -54,7 +50,7 @@
      16    function testFunctions() {
  ->  17        |debugger;
      18        let before = 1;
-     19        testAlert("function log 1");
+     19        a();
      20        let after = 2;
 
 PAUSE AT testFunctions:19:5
@@ -62,7 +58,7 @@
      16    function testFunctions() {
      17        debugger;
  ->  18        |let before = 1;
-     19        testAlert("function log 1");
+     19        a();
      20        let after = 2;
      21    }
 
@@ -70,7 +66,7 @@
      16    function testFunctions() {
      17        debugger;
      18        let before = 1;
- ->  19        |testAlert("function log 1");
+ ->  19        |a();
      20        let after = 2;
      21    }
      22    
@@ -78,7 +74,7 @@
 PAUSE AT testFunctions:21:5
      17        debugger;
      18        let before = 1;
-     19        testAlert("function log 1");
+     19        a();
  ->  20        |let after = 2;
      21    }
      22    
@@ -86,7 +82,7 @@
 
 PAUSE AT testFunctions:22:2
      18        let before = 1;
-     19        testAlert("function log 1");
+     19        a();
      20        let after = 2;
  ->  21    }|
      22    
@@ -200,72 +196,108 @@
      37    
      38    function testCommas() {
  ->  39        |debugger;
-     40        let a = 1,
-     41            b = 2,
-     42            c = 3;
+     40        let x = 1,
+     41            y = 2,
+     42            z = 3;
 
 PAUSE AT testCommas:41:5
      37    
      38    function testCommas() {
      39        debugger;
- ->  40        |let a = 1,
-     41            b = 2,
-     42            c = 3;
-     43        testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
+ ->  40        |let x = 1,
+     41            y = 2,
+     42            z = 3;
+     43        a(), b(), c();
 
 PAUSE AT testCommas:42:9
      38    function testCommas() {
      39        debugger;
-     40        let a = 1,
- ->  41            |b = 2,
-     42            c = 3;
-     43        testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
-     44    }
+     40        let x = 1,
+ ->  41            |y = 2,
+     42            z = 3;
+     43        a(), b(), c();
+     44        true && (a(), b(), c());
 
 PAUSE AT testCommas:43:9
      39        debugger;
-     40        let a = 1,
-     41            b = 2,
- ->  42            |c = 3;
-     43        testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
-     44    }
-     45    
+     40        let x = 1,
+     41            y = 2,
+ ->  42            |z = 3;
+     43        a(), b(), c();
+     44        true && (a(), b(), c());
+     45    }
 
 PAUSE AT testCommas:44:5
-     40        let a = 1,
-     41            b = 2,
-     42            c = 3;
- ->  43        |testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
-     44    }
-     45    
-     46    // ---------
+     40        let x = 1,
+     41            y = 2,
+     42            z = 3;
+ ->  43        |a(), b(), c();
+     44        true && (a(), b(), c());
+     45    }
+     46    
 
-PAUSE AT testCommas:44:31
-     40        let a = 1,
-     41            b = 2,
-     42            c = 3;
- ->  43        testAlert("comma log 1"), |testAlert("comma log 2"), testAlert("comma log 3");
-     44    }
-     45    
-     46    // ---------
+PAUSE AT testCommas:44:10
+     40        let x = 1,
+     41            y = 2,
+     42            z = 3;
+ ->  43        a(), |b(), c();
+     44        true && (a(), b(), c());
+     45    }
+     46    
 
-PAUSE AT testCommas:44:57
-     40        let a = 1,
-     41            b = 2,
-     42            c = 3;
- ->  43        testAlert("comma log 1"), testAlert("comma log 2"), |testAlert("comma log 3");
-     44    }
-     45    
-     46    // ---------
+PAUSE AT testCommas:44:15
+     40        let x = 1,
+     41            y = 2,
+     42            z = 3;
+ ->  43        a(), b(), |c();
+     44        true && (a(), b(), c());
+     45    }
+     46    
 
-PAUSE AT testCommas:45:2
-     41            b = 2,
-     42            c = 3;
-     43        testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
- ->  44    }|
-     45    
-     46    // ---------
-     47    
+PAUSE AT testCommas:45:5
+     41            y = 2,
+     42            z = 3;
+     43        a(), b(), c();
+ ->  44        |true && (a(), b(), c());
+     45    }
+     46    
+     47    function a() { }
 
+PAUSE AT testCommas:45:14
+     41            y = 2,
+     42            z = 3;
+     43        a(), b(), c();
+ ->  44        true && (|a(), b(), c());
+     45    }
+     46    
+     47    function a() { }
+
+PAUSE AT testCommas:45:19
+     41            y = 2,
+     42            z = 3;
+     43        a(), b(), c();
+ ->  44        true && (a(), |b(), c());
+     45    }
+     46    
+     47    function a() { }
+
+PAUSE AT testCommas:45:24
+     41            y = 2,
+     42            z = 3;
+     43        a(), b(), c();
+ ->  44        true && (a(), b(), |c());
+     45    }
+     46    
+     47    function a() { }
+
+PAUSE AT testCommas:46:2
+     42            z = 3;
+     43        a(), b(), c();
+     44        true && (a(), b(), c());
+ ->  45    }|
+     46    
+     47    function a() { }
+     48    function b() { }
+
 RESUMED
 

Modified: trunk/LayoutTests/inspector/debugger/stepping/stepOver.html (259809 => 259810)


--- trunk/LayoutTests/inspector/debugger/stepping/stepOver.html	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/inspector/debugger/stepping/stepOver.html	2020-04-09 18:22:00 UTC (rev 259810)
@@ -17,7 +17,7 @@
 function testFunctions() {
     debugger;
     let before = 1;
-    testAlert("function log 1");
+    a();
     let after = 2;
 }
 
@@ -38,12 +38,17 @@
 
 function testCommas() {
     debugger;
-    let a = 1,
-        b = 2,
-        c = 3;
-    testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
+    let x = 1,
+        y = 2,
+        z = 3;
+    a(), b(), c();
+    true && (a(), b(), c());
 }
 
+function a() { }
+function b() { }
+function c() { }
+
 // ---------
 
 function test()

Modified: trunk/LayoutTests/inspector/formatting/formatting-_javascript_-expected.txt (259809 => 259810)


--- trunk/LayoutTests/inspector/formatting/formatting-_javascript_-expected.txt	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/inspector/formatting/formatting-_javascript_-expected.txt	2020-04-09 18:22:00 UTC (rev 259810)
@@ -5,6 +5,7 @@
 -- Running test case: JSFormatter
 PASS: arrow-functions.js
 PASS: classes.js
+PASS: comma-expressions.js
 PASS: comments-and-preserve-newlines.js
 PASS: comments-only.js
 PASS: do-while-statement.js

Modified: trunk/LayoutTests/inspector/formatting/formatting-_javascript_.html (259809 => 259810)


--- trunk/LayoutTests/inspector/formatting/formatting-_javascript_.html	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/inspector/formatting/formatting-_javascript_.html	2020-04-09 18:22:00 UTC (rev 259810)
@@ -11,6 +11,7 @@
     addFormattingTests(suite, "text/_javascript_", [
         "resources/_javascript_-tests/arrow-functions.js",
         "resources/_javascript_-tests/classes.js",
+        "resources/_javascript_-tests/comma-expressions.js",
         "resources/_javascript_-tests/comments-and-preserve-newlines.js",
         "resources/_javascript_-tests/comments-only.js",
         "resources/_javascript_-tests/do-while-statement.js",

Modified: trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/arrow-functions-expected.js (259809 => 259810)


--- trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/arrow-functions-expected.js	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/arrow-functions-expected.js	2020-04-09 18:22:00 UTC (rev 259810)
@@ -1,12 +1,20 @@
 x => x;
 x => x * x;
-x => {x * x};
-x => {x * x;};
+x => {
+    x * x
+};
+x => {
+    x * x;
+};
 () => 1;
 (x) => x;
 (x) => x * x;
-(x) => {x * x};
-(x) => {x * x;};
+(x) => {
+    x * x
+};
+(x) => {
+    x * x;
+};
 
 x => {
     x *= x;
@@ -36,7 +44,9 @@
 
 async x => x
 async (x) => x
-async (x) => {x}
+async (x) => {
+    x
+}
 
 a => {
     for (b of [])

Added: trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/comma-expressions-expected.js (0 => 259810)


--- trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/comma-expressions-expected.js	                        (rev 0)
+++ trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/comma-expressions-expected.js	2020-04-09 18:22:00 UTC (rev 259810)
@@ -0,0 +1,39 @@
+a(x, y, z),
+b(x, y, z),
+c(x, y, z);
+
+if (a(x, y, z), b(x, y, z), c(x, y, z)) {}
+
+true && (a(x, y, z), b(x, y, z), c(x, y, z)) && true;
+
+(x, y, z) => {
+    a(x, y, z),
+    b(x, y, z),
+    c(x, y, z)
+}
+
+(x, y, z) => {
+    if (a(x, y, z), b(x, y, z), c(x, y, z)) {}
+}
+
+(x, y, z) => {
+    true && (a(x, y, z), b(x, y, z), c(x, y, z)) && true;
+}
+
+(x, y, z) => (a(x, y, z), b(x, y, z), c(x, y, z));
+
+(x, y, z) => true && (a(x, y, z), b(x, y, z), c(x, y, z)) && true;
+
+function foo(x, y, z) {
+    a(x, y, z),
+    b(x, y, z),
+    c(x, y, z)
+}
+
+function foo(x, y, z) {
+    if (a(x, y, z), b(x, y, z), c(x, y, z)) {}
+}
+
+function foo(x, y, z) {
+    true && (a(x, y, z), b(x, y, z), c(x, y, z)) && true;
+}

Added: trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/comma-expressions.js (0 => 259810)


--- trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/comma-expressions.js	                        (rev 0)
+++ trunk/LayoutTests/inspector/formatting/resources/_javascript_-tests/comma-expressions.js	2020-04-09 18:22:00 UTC (rev 259810)
@@ -0,0 +1,21 @@
+a(x, y, z), b(x, y, z), c(x, y, z);
+
+if (a(x, y, z), b(x, y, z), c(x, y, z)) { }
+
+true && (a(x, y, z), b(x, y, z), c(x, y, z)) && true;
+
+(x, y, z) => { a(x, y, z), b(x, y, z), c(x, y, z) }
+
+(x, y, z) => { if (a(x, y, z), b(x, y, z), c(x, y, z)) { } }
+
+(x, y, z) => { true && (a(x, y, z), b(x, y, z), c(x, y, z)) && true; }
+
+(x, y, z) => (a(x, y, z), b(x, y, z), c(x, y, z));
+
+(x, y, z) => true && (a(x, y, z), b(x, y, z), c(x, y, z)) && true;
+
+function foo(x, y, z) { a(x, y, z), b(x, y, z), c(x, y, z) }
+
+function foo(x, y, z) { if (a(x, y, z), b(x, y, z), c(x, y, z)) { } }
+
+function foo(x, y, z) { true && (a(x, y, z), b(x, y, z), c(x, y, z)) && true; }

Modified: trunk/Source/_javascript_Core/ChangeLog (259809 => 259810)


--- trunk/Source/_javascript_Core/ChangeLog	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-04-09 18:22:00 UTC (rev 259810)
@@ -1,3 +1,26 @@
+2020-04-09  Devin Rousso  <drou...@apple.com>
+
+        Web Inspector: Debugger: debug hooks should also be emitted for the first sub-_expression_ in a comma _expression_
+        https://bugs.webkit.org/show_bug.cgi?id=210253
+
+        Reviewed by Joseph Pecoraro.
+
+        * bytecompiler/NodesCodegen.cpp:
+        (JSC::CommaNode::emitBytecode):
+        * parser/Parser.cpp:
+        (JSC::Parser<LexerType>::parseVariableDeclarationList):
+        (JSC::Parser<LexerType>::parseExpression):
+        We should emit debug hooks and record pause locations for the first sub-_expression_ in comma
+        expressions, as the comma _expression_ is not always standalone (e.g. `true && (a(), b())`).
+
+        * bytecompiler/BytecodeGenerator.h:
+        * bytecompiler/BytecodeGenerator.cpp:
+        (JSC::BytecodeGenerator::emitDebugHook):
+        Save the `JSTextPosition` and `DebugHookType` of the last debug hook, using them to prevent
+        any additional debug hooks from being emitted if they have the same `JSTextPosition` and
+        `DebugHookType`. This prevents the debugger from pausing twice at the beginning of an
+        _expression_ statement (e.g. `|a(), b();`).
+
 2020-04-09  Saam Barati  <sbar...@apple.com>
 
         We can still cache delete in strict mode as long as the property is not "non-configurable"

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp (259809 => 259810)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.cpp	2020-04-09 18:22:00 UTC (rev 259810)
@@ -3535,9 +3535,15 @@
 
 void BytecodeGenerator::emitDebugHook(DebugHookType debugHookType, const JSTextPosition& divot)
 {
-    if (!shouldEmitDebugHooks())
+    if (LIKELY(!shouldEmitDebugHooks()))
         return;
 
+    if (m_lastDebugHook.position == divot && m_lastDebugHook.type == debugHookType)
+        return;
+
+    m_lastDebugHook.position = divot;
+    m_lastDebugHook.type = debugHookType;
+
     emitExpressionInfo(divot, divot, divot);
     OpDebug::emit(this, debugHookType, false);
 }

Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (259809 => 259810)


--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h	2020-04-09 18:22:00 UTC (rev 259810)
@@ -1312,6 +1312,11 @@
             VirtualRegister completionTypeRegister;
         };
         Vector<CatchEntry> m_exceptionHandlersToEmit;
+
+        struct {
+            JSTextPosition position;
+            DebugHookType type { DidExecuteProgram };
+        } m_lastDebugHook;
     };
 
     class StrictModeScope : private SetForScope<ECMAMode> {

Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (259809 => 259810)


--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2020-04-09 18:22:00 UTC (rev 259810)
@@ -3043,12 +3043,10 @@
 {
     CommaNode* node = this;
     for (; node->next(); node = node->next()) {
+        generator.emitDebugHook(node->m_expr);
         generator.emitNode(generator.ignoredResult(), node->m_expr);
-
-        // Don't emit a debug hook for the first _expression_, as that should've already happened in
-        // the containing statement.
-        generator.emitDebugHook(node->next()->m_expr);
     }
+    generator.emitDebugHook(node->m_expr);
     return generator.emitNodeInTailPosition(dst, node->m_expr);
 }
 

Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (259809 => 259810)


--- trunk/Source/_javascript_Core/parser/Parser.cpp	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp	2020-04-09 18:22:00 UTC (rev 259810)
@@ -897,8 +897,10 @@
                 head = node;
                 headLocation = location;
             } else {
-                if (!tail)
+                if (!tail) {
                     head = tail = context.createCommaExpr(headLocation, head);
+                    recordPauseLocation(context.breakpointLocation(head));
+                }
                 tail = context.appendToCommaExpr(location, head, tail, node);
                 recordPauseLocation(context.breakpointLocation(tail));
             }
@@ -3718,6 +3720,7 @@
     failIfFalse(right, "Cannot parse _expression_ in a comma _expression_");
     context.setEndOffset(right, m_lastTokenEndPosition.offset);
     typename TreeBuilder::Comma head = context.createCommaExpr(headLocation, node);
+    recordPauseLocation(context.breakpointLocation(head));
     typename TreeBuilder::Comma tail = context.appendToCommaExpr(tailLocation, head, head, right);
     recordPauseLocation(context.breakpointLocation(tail));
     while (match(COMMA)) {

Modified: trunk/Source/WebInspectorUI/ChangeLog (259809 => 259810)


--- trunk/Source/WebInspectorUI/ChangeLog	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/Source/WebInspectorUI/ChangeLog	2020-04-09 18:22:00 UTC (rev 259810)
@@ -1,3 +1,16 @@
+2020-04-09  Devin Rousso  <drou...@apple.com>
+
+        Web Inspector: Debugger: debug hooks should also be emitted for the first sub-_expression_ in a comma _expression_
+        https://bugs.webkit.org/show_bug.cgi?id=210253
+
+        Reviewed by Joseph Pecoraro.
+
+        * UserInterface/Workers/Formatter/JSFormatter.js:
+        (JSFormatter.prototype._handleTokenAtNode):
+        (JSFormatter.prototype._isLikelyToHaveNewline): Deleted.
+        If an arrow function wraps it's body with `{` and `}`, always add newlines to make setting
+        breakpoints inside the function body easier.
+
 2020-04-08  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: Debugger: treat comma sub-expressions as separate statements

Modified: trunk/Source/WebInspectorUI/UserInterface/Workers/Formatter/JSFormatter.js (259809 => 259810)


--- trunk/Source/WebInspectorUI/UserInterface/Workers/Formatter/JSFormatter.js	2020-04-09 18:17:56 UTC (rev 259809)
+++ trunk/Source/WebInspectorUI/UserInterface/Workers/Formatter/JSFormatter.js	2020-04-09 18:22:00 UTC (rev 259810)
@@ -199,30 +199,6 @@
         return (parent.type === "ForStatement" || parent.type === "ForInStatement" || parent.type === "ForOfStatement") && node !== parent.body;
     }
 
-    _isLikelyToHaveNewline(node)
-    {
-        switch (node.type) {
-        case "BlockStatement":
-        case "ClassDeclaration":
-        case "DoWhileStatement":
-        case "ForInStatement":
-        case "ForOfStatement":
-        case "ForStatement":
-        case "FunctionDeclaration":
-        case "IfStatement":
-        case "SwitchStatement":
-        case "TryStatement":
-        case "WhileStatement":
-        case "WithStatement":
-            return true;
-
-        case "ExpressionStatement":
-            return node._expression_.type === "SequenceExpression";
-        }
-
-        return false;
-    }
-
     _isRangeWhitespace(from, to)
     {
         let substring = this._sourceText.substring(from, to);
@@ -332,7 +308,6 @@
         }
 
         if (nodeType === "BlockStatement") {
-            let isSingleStatementArrowFunctionWithUnlikelyMultilineContent = node.parent.type === "ArrowFunctionExpression" && node.body.length === 1 && !this._isLikelyToHaveNewline(node.body[0]);
             if (tokenValue === "{") {
                 // Class methods we put the opening brace on its own line.
                 if (node.parent && node.parent.parent && node.parent.parent.type === "MethodDefinition" && node.body.length) {
@@ -343,13 +318,13 @@
                     return;
                 }
                 builder.appendToken(tokenValue, tokenOffset);
-                if (node.body.length && !isSingleStatementArrowFunctionWithUnlikelyMultilineContent)
+                if (node.body.length)
                     this._appendNewline(node);
                 builder.indent();
                 return;
             }
             if (tokenValue === "}") {
-                if (node.body.length && !isSingleStatementArrowFunctionWithUnlikelyMultilineContent)
+                if (node.body.length)
                     this._appendNewline(node);
                 builder.dedent();
                 builder.appendToken(tokenValue, tokenOffset);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to