⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 259810 in webkit


Ignore:
Timestamp:
Apr 9, 2020, 11:22:00 AM (6 years ago)
Author:
Devin Rousso
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/JavaScriptCore:

  • 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.
Location:
trunk
Files:
2 added
16 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r259805 r259810  
     12020-04-09  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: Debugger: debug hooks should also be emitted for the first sub-expression in a comma expression
     4        https://bugs.webkit.org/show_bug.cgi?id=210253
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        * inspector/debugger/breakpoints/resources/dump-general.js:
     9        * inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt:
     10        * inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt:
     11        * inspector/debugger/stepping/stepOver.html:
     12        * inspector/debugger/stepping/stepOver-expected.txt:
     13        * inspector/formatting/formatting-javascript.html:
     14        * inspector/formatting/formatting-javascript-expected.txt:
     15        * inspector/formatting/resources/javascript-tests/arrow-functions-expected.js:
     16        * inspector/formatting/resources/javascript-tests/comma-expressions.js: Added.
     17        * inspector/formatting/resources/javascript-tests/comma-expressions-expected.js: Added.
     18
    1192020-04-08  Simon Fraser  <simon.fraser@apple.com>
    220

  • trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt

    r259781 r259810  
    14121412    215    c();
    14131413    216   
     1414    217    true && (a(), b(), c());
    14141415
    14151416INSERTING AT: 214:1
    … …  
    14211422 => 215    |c();
    14221423    216   
     1424    217    true && (a(), b(), c());
     1425    218   
     1426
     1427INSERTING AT: 215:1
     1428PAUSES AT: 217:0
     1429    212   
     1430    213    a(),
     1431    214    b(),
     1432 -> 215    c#();
     1433    216   
     1434 => 217    |true && (a(), b(), c());
     1435    218   
     1436
     1437INSERTING AT: 217:1
     1438PAUSES AT: 217:9
     1439    214    b(),
     1440    215    c();
     1441    216   
     1442-=> 217    t#rue && (|a(), b(), c());
     1443    218   
     1444
     1445INSERTING AT: 217:10
     1446PAUSES AT: 217:14
     1447    214    b(),
     1448    215    c();
     1449    216   
     1450-=> 217    true && (a#(), |b(), c());
     1451    218   
     1452
     1453INSERTING AT: 217:15
     1454PAUSES AT: 217:19
     1455    214    b(),
     1456    215    c();
     1457    216   
     1458-=> 217    true && (a(), b#(), |c());
     1459    218   
    14231460
    14241461

  • trunk/LayoutTests/inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt

    r259781 r259810  
    30943094    215    c();
    30953095    216   
     3096    217    true && (a(), b(), c());
    30963097
    30973098
    … …  
    31033104-=> 215    |c();
    31043105    216   
     3106    217    true && (a(), b(), c());
     3107    218   
    31053108
    31063109
    31073110INSERTING AT: 216:0
     3111PAUSES AT: 217:0
     3112    213    a(),
     3113    214    b(),
     3114    215    c();
     3115 -> 216    #
     3116 => 217    |true && (a(), b(), c());
     3117    218   
     3118
     3119
     3120INSERTING AT: 217:0
     3121PAUSES AT: 217:0
     3122    214    b(),
     3123    215    c();
     3124    216   
     3125-=> 217    |true && (a(), b(), c());
     3126    218   
     3127
     3128
     3129INSERTING AT: 218:0
    31083130PRODUCES: Could not resolve breakpoint
    31093131

  • trunk/LayoutTests/inspector/debugger/breakpoints/resources/dump-general.js

    r259781 r259810  
    215215b(),
    216216c();
     217
     218true && (a(), b(), c());

  • trunk/LayoutTests/inspector/debugger/stepping/stepOver-expected.txt

    r259781 r259810  
    1 ALERT: function log 1
    2 ALERT: comma log 1
    3 ALERT: comma log 2
    4 ALERT: comma log 3
    51Checking pause locations when stepping with "stepOver".
    62
    … …  
    5551 ->  17        |debugger;
    5652     18        let before = 1;
    57      19        testAlert("function log 1");
     53     19        a();
    5854     20        let after = 2;
    5955
    … …  
    6359     17        debugger;
    6460 ->  18        |let before = 1;
    65      19        testAlert("function log 1");
     61     19        a();
    6662     20        let after = 2;
    6763     21    }
    … …  
    7167     17        debugger;
    7268     18        let before = 1;
    73  ->  19        |testAlert("function log 1");
     69 ->  19        |a();
    7470     20        let after = 2;
    7571     21    }
    … …  
    7975     17        debugger;
    8076     18        let before = 1;
    81      19        testAlert("function log 1");
     77     19        a();
    8278 ->  20        |let after = 2;
    8379     21    }
    … …  
    8783PAUSE AT testFunctions:22:2
    8884     18        let before = 1;
    89      19        testAlert("function log 1");
     85     19        a();
    9086     20        let after = 2;
    9187 ->  21    }|
    … …  
    201197     38    function testCommas() {
    202198 ->  39        |debugger;
    203      40        let a = 1,
    204      41            b = 2,
    205      42            c = 3;
     199     40        let x = 1,
     200     41            y = 2,
     201     42            z = 3;
    206202
    207203PAUSE AT testCommas:41:5
    … …  
    209205     38    function testCommas() {
    210206     39        debugger;
    211  ->  40        |let a = 1,
    212      41            b = 2,
    213      42            c = 3;
    214      43        testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
     207 ->  40        |let x = 1,
     208     41            y = 2,
     209     42            z = 3;
     210     43        a(), b(), c();
    215211
    216212PAUSE AT testCommas:42:9
    217213     38    function testCommas() {
    218214     39        debugger;
    219      40        let a = 1,
    220  ->  41            |b = 2,
    221      42            c = 3;
    222      43        testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
    223      44    }
     215     40        let x = 1,
     216 ->  41            |y = 2,
     217     42            z = 3;
     218     43        a(), b(), c();
     219     44        true && (a(), b(), c());
    224220
    225221PAUSE AT testCommas:43:9
    226222     39        debugger;
    227      40        let a = 1,
    228      41            b = 2,
    229  ->  42            |c = 3;
    230      43        testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
    231      44    }
    232      45   
     223     40        let x = 1,
     224     41            y = 2,
     225 ->  42            |z = 3;
     226     43        a(), b(), c();
     227     44        true && (a(), b(), c());
     228     45    }
    233229
    234230PAUSE AT testCommas:44:5
    235      40        let a = 1,
    236      41            b = 2,
    237      42            c = 3;
    238  ->  43        |testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
    239      44    }
    240      45   
    241      46    // ---------
    242 
    243 PAUSE AT testCommas:44:31
    244      40        let a = 1,
    245      41            b = 2,
    246      42            c = 3;
    247  ->  43        testAlert("comma log 1"), |testAlert("comma log 2"), testAlert("comma log 3");
    248      44    }
    249      45   
    250      46    // ---------
    251 
    252 PAUSE AT testCommas:44:57
    253      40        let a = 1,
    254      41            b = 2,
    255      42            c = 3;
    256  ->  43        testAlert("comma log 1"), testAlert("comma log 2"), |testAlert("comma log 3");
    257      44    }
    258      45   
    259      46    // ---------
    260 
    261 PAUSE AT testCommas:45:2
    262      41            b = 2,
    263      42            c = 3;
    264      43        testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
    265  ->  44    }|
    266      45   
    267      46    // ---------
    268      47   
    269 
    270 RESUMED
    271 
     231     40        let x = 1,
     232     41            y = 2,
     233     42            z = 3;
     234 ->  43        |a(), b(), c();
     235     44        true && (a(), b(), c());
     236     45    }
     237     46   
     238
     239PAUSE AT testCommas:44:10
     240     40        let x = 1,
     241     41            y = 2,
     242     42            z = 3;
     243 ->  43        a(), |b(), c();
     244     44        true && (a(), b(), c());
     245     45    }
     246     46   
     247
     248PAUSE AT testCommas:44:15
     249     40        let x = 1,
     250     41            y = 2,
     251     42            z = 3;
     252 ->  43        a(), b(), |c();
     253     44        true && (a(), b(), c());
     254     45    }
     255     46   
     256
     257PAUSE AT testCommas:45:5
     258     41            y = 2,
     259     42            z = 3;
     260     43        a(), b(), c();
     261 ->  44        |true && (a(), b(), c());
     262     45    }
     263     46   
     264     47    function a() { }
     265
     266PAUSE AT testCommas:45:14
     267     41            y = 2,
     268     42            z = 3;
     269     43        a(), b(), c();
     270 ->  44        true && (|a(), b(), c());
     271     45    }
     272     46   
     273     47    function a() { }
     274
     275PAUSE AT testCommas:45:19
     276     41            y = 2,
     277     42            z = 3;
     278     43        a(), b(), c();
     279 ->  44        true && (a(), |b(), c());
     280     45    }
     281     46   
     282     47    function a() { }
     283
     284PAUSE AT testCommas:45:24
     285     41            y = 2,
     286     42            z = 3;
     287     43        a(), b(), c();
     288 ->  44        true && (a(), b(), |c());
     289     45    }
     290     46   
     291     47    function a() { }
     292
     293PAUSE AT testCommas:46:2
     294     42            z = 3;
     295     43        a(), b(), c();
     296     44        true && (a(), b(), c());
     297 ->  45    }|
     298     46   
     299     47    function a() { }
     300     48    function b() { }
     301
     302RESUMED
     303

  • trunk/LayoutTests/inspector/debugger/stepping/stepOver.html

    r259781 r259810  
    1818    debugger;
    1919    let before = 1;
    20     testAlert("function log 1");
     20    a();
    2121    let after = 2;
    2222}
    … …  
    3939function testCommas() {
    4040    debugger;
    41     let a = 1,
    42         b = 2,
    43         c = 3;
    44     testAlert("comma log 1"), testAlert("comma log 2"), testAlert("comma log 3");
     41    let x = 1,
     42        y = 2,
     43        z = 3;
     44    a(), b(), c();
     45    true && (a(), b(), c());
    4546}
     47
     48function a() { }
     49function b() { }
     50function c() { }
    4651
    4752// ---------

  • trunk/LayoutTests/inspector/formatting/formatting-javascript-expected.txt

    r249831 r259810  
    66PASS: arrow-functions.js
    77PASS: classes.js
     8PASS: comma-expressions.js
    89PASS: comments-and-preserve-newlines.js
    910PASS: comments-only.js

  • trunk/LayoutTests/inspector/formatting/formatting-javascript.html

    r249831 r259810  
    1212        "resources/javascript-tests/arrow-functions.js",
    1313        "resources/javascript-tests/classes.js",
     14        "resources/javascript-tests/comma-expressions.js",
    1415        "resources/javascript-tests/comments-and-preserve-newlines.js",
    1516        "resources/javascript-tests/comments-only.js",

  • trunk/LayoutTests/inspector/formatting/resources/javascript-tests/arrow-functions-expected.js

    r248785 r259810  
    11x => x;
    22x => x * x;
    3 x => {x * x};
    4 x => {x * x;};
     3x => {
     4    x * x
     5};
     6x => {
     7    x * x;
     8};
    59() => 1;
    610(x) => x;
    711(x) => x * x;
    8 (x) => {x * x};
    9 (x) => {x * x;};
     12(x) => {
     13    x * x
     14};
     15(x) => {
     16    x * x;
     17};
    1018
    1119x => {
    … …  
    3745async x => x
    3846async (x) => x
    39 async (x) => {x}
     47async (x) => {
     48    x
     49}
    4050
    4151a => {

  • trunk/Source/JavaScriptCore/ChangeLog

    r259807 r259810  
     12020-04-09  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: Debugger: debug hooks should also be emitted for the first sub-expression in a comma expression
     4        https://bugs.webkit.org/show_bug.cgi?id=210253
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        * bytecompiler/NodesCodegen.cpp:
     9        (JSC::CommaNode::emitBytecode):
     10        * parser/Parser.cpp:
     11        (JSC::Parser<LexerType>::parseVariableDeclarationList):
     12        (JSC::Parser<LexerType>::parseExpression):
     13        We should emit debug hooks and record pause locations for the first sub-expression in comma
     14        expressions, as the comma expression is not always standalone (e.g. `true && (a(), b())`).
     15
     16        * bytecompiler/BytecodeGenerator.h:
     17        * bytecompiler/BytecodeGenerator.cpp:
     18        (JSC::BytecodeGenerator::emitDebugHook):
     19        Save the `JSTextPosition` and `DebugHookType` of the last debug hook, using them to prevent
     20        any additional debug hooks from being emitted if they have the same `JSTextPosition` and
     21        `DebugHookType`. This prevents the debugger from pausing twice at the beginning of an
     22        expression statement (e.g. `|a(), b();`).
     23
    1242020-04-09  Saam Barati  <sbarati@apple.com>
    225

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r259676 r259810  
    35363536void BytecodeGenerator::emitDebugHook(DebugHookType debugHookType, const JSTextPosition& divot)
    35373537{
    3538     if (!shouldEmitDebugHooks())
     3538    if (LIKELY(!shouldEmitDebugHooks()))
    35393539        return;
     3540
     3541    if (m_lastDebugHook.position == divot && m_lastDebugHook.type == debugHookType)
     3542        return;
     3543
     3544    m_lastDebugHook.position = divot;
     3545    m_lastDebugHook.type = debugHookType;
    35403546
    35413547    emitExpressionInfo(divot, divot, divot);

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r259676 r259810  
    13131313        };
    13141314        Vector<CatchEntry> m_exceptionHandlersToEmit;
     1315
     1316        struct {
     1317            JSTextPosition position;
     1318            DebugHookType type { DidExecuteProgram };
     1319        } m_lastDebugHook;
    13151320    };
    13161321

  • trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

    r259781 r259810  
    30443044    CommaNode* node = this;
    30453045    for (; node->next(); node = node->next()) {
     3046        generator.emitDebugHook(node->m_expr);
    30463047        generator.emitNode(generator.ignoredResult(), node->m_expr);
    3047 
    3048         // Don't emit a debug hook for the first expression, as that should've already happened in
    3049         // the containing statement.
    3050         generator.emitDebugHook(node->next()->m_expr);
    3051     }
     3048    }
     3049    generator.emitDebugHook(node->m_expr);
    30523050    return generator.emitNodeInTailPosition(dst, node->m_expr);
    30533051}

  • trunk/Source/JavaScriptCore/parser/Parser.cpp

    r259781 r259810  
    898898                headLocation = location;
    899899            } else {
    900                 if (!tail)
     900                if (!tail) {
    901901                    head = tail = context.createCommaExpr(headLocation, head);
     902                    recordPauseLocation(context.breakpointLocation(head));
     903                }
    902904                tail = context.appendToCommaExpr(location, head, tail, node);
    903905                recordPauseLocation(context.breakpointLocation(tail));
    … …  
    37193721    context.setEndOffset(right, m_lastTokenEndPosition.offset);
    37203722    typename TreeBuilder::Comma head = context.createCommaExpr(headLocation, node);
     3723    recordPauseLocation(context.breakpointLocation(head));
    37213724    typename TreeBuilder::Comma tail = context.appendToCommaExpr(tailLocation, head, head, right);
    37223725    recordPauseLocation(context.breakpointLocation(tail));

  • trunk/Source/WebInspectorUI/ChangeLog

    r259781 r259810  
     12020-04-09  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: Debugger: debug hooks should also be emitted for the first sub-expression in a comma expression
     4        https://bugs.webkit.org/show_bug.cgi?id=210253
     5
     6        Reviewed by Joseph Pecoraro.
     7
     8        * UserInterface/Workers/Formatter/JSFormatter.js:
     9        (JSFormatter.prototype._handleTokenAtNode):
     10        (JSFormatter.prototype._isLikelyToHaveNewline): Deleted.
     11        If an arrow function wraps it's body with `{` and `}`, always add newlines to make setting
     12        breakpoints inside the function body easier.
     13
    1142020-04-08  Devin Rousso  <drousso@apple.com>
    215

  • trunk/Source/WebInspectorUI/UserInterface/Workers/Formatter/JSFormatter.js

    r259781 r259810  
    200200    }
    201201
    202     _isLikelyToHaveNewline(node)
    203     {
    204         switch (node.type) {
    205         case "BlockStatement":
    206         case "ClassDeclaration":
    207         case "DoWhileStatement":
    208         case "ForInStatement":
    209         case "ForOfStatement":
    210         case "ForStatement":
    211         case "FunctionDeclaration":
    212         case "IfStatement":
    213         case "SwitchStatement":
    214         case "TryStatement":
    215         case "WhileStatement":
    216         case "WithStatement":
    217             return true;
    218 
    219         case "ExpressionStatement":
    220             return node.expression.type === "SequenceExpression";
    221         }
    222 
    223         return false;
    224     }
    225 
    226202    _isRangeWhitespace(from, to)
    227203    {
    … …  
    333309
    334310        if (nodeType === "BlockStatement") {
    335             let isSingleStatementArrowFunctionWithUnlikelyMultilineContent = node.parent.type === "ArrowFunctionExpression" && node.body.length === 1 && !this._isLikelyToHaveNewline(node.body[0]);
    336311            if (tokenValue === "{") {
    337312                // Class methods we put the opening brace on its own line.
    … …  
    344319                }
    345320                builder.appendToken(tokenValue, tokenOffset);
    346                 if (node.body.length && !isSingleStatementArrowFunctionWithUnlikelyMultilineContent)
     321                if (node.body.length)
    347322                    this._appendNewline(node);
    348323                builder.indent();
    … …  
    350325            }
    351326            if (tokenValue === "}") {
    352                 if (node.body.length && !isSingleStatementArrowFunctionWithUnlikelyMultilineContent)
     327                if (node.body.length)
    353328                    this._appendNewline(node);
    354329                builder.dedent();
Note: See TracChangeset for help on using the changeset viewer.