Changeset 207312 in webkit


Ignore:
Timestamp:
Oct 13, 2016 3:20:22 PM (8 years ago)
Author:
commit-queue@webkit.org
Message:

Web Inspector: Stepping highlight for dot/bracket expressions in if statements highlights subset of the expression
​https://bugs.webkit.org/show_bug.cgi?id=163378
<rdar://problem/28749376>

Patch by Joseph Pecoraro <Joseph Pecoraro> on 2016-10-13
Reviewed by Saam Barati.

Source/JavaScriptCore:

  • parser/Parser.cpp:

(JSC::Parser<LexerType>::parseAssignmentExpression):
Since each expression builds on the previous, always keep the starting
location the first location.

LayoutTests:

  • inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt:
  • inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt:
  • inspector/debugger/breakpoints/resources/dump-general.js:
  • inspector/debugger/stepping/stepping-misc-expected.txt:
  • inspector/debugger/stepping/stepping-misc.html:

Add tests for these kinds of special cases.

Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r207299 r207312  
     12016-10-13  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Web Inspector: Stepping highlight for dot/bracket expressions in if statements highlights subset of the expression
     4        https://bugs.webkit.org/show_bug.cgi?id=163378
     5        <rdar://problem/28749376>
     6
     7        Reviewed by Saam Barati.
     8
     9        * inspector/debugger/breakpoints/resolved-dump-all-pause-locations-expected.txt:
     10        * inspector/debugger/breakpoints/resolved-dump-each-line-expected.txt:
     11        * inspector/debugger/breakpoints/resources/dump-general.js:
     12        * inspector/debugger/stepping/stepping-misc-expected.txt:
     13        * inspector/debugger/stepping/stepping-misc.html:
     14        Add tests for these kinds of special cases.
     15
    1162016-10-13  Antoine Quint  <graouts@apple.com>
    217

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

    r206653 r207312  
    12571257 => 200    |a(a(), b());
    12581258    201   
     1259    202    if (o1.p1)
     1260    203        a();
     1261
     1262INSERTING AT: 200:1
     1263PAUSES AT: 202:4
     1264    197    var t1 = `${1} ${x=1} ${a()}`;
     1265    198    var t2 = a`${1} ${x=1} ${a()}`;
     1266    199   
     1267 -> 200    a#(a(), b());
     1268    201   
     1269 => 202    if (|o1.p1)
     1270    203        a();
     1271    204   
     1272    205    if (o1["p1"])
     1273
     1274INSERTING AT: 202:5
     1275PAUSES AT: 203:4
     1276    199   
     1277    200    a(a(), b());
     1278    201   
     1279 -> 202    if (o#1.p1)
     1280 => 203        |a();
     1281    204   
     1282    205    if (o1["p1"])
     1283    206        a();
     1284
     1285INSERTING AT: 203:5
     1286PAUSES AT: 205:4
     1287    200    a(a(), b());
     1288    201   
     1289    202    if (o1.p1)
     1290 -> 203        a#();
     1291    204   
     1292 => 205    if (|o1["p1"])
     1293    206        a();
     1294    207   
     1295    208    if (String.raw`test`)
     1296
     1297INSERTING AT: 205:5
     1298PAUSES AT: 206:4
     1299    202    if (o1.p1)
     1300    203        a();
     1301    204   
     1302 -> 205    if (o#1["p1"])
     1303 => 206        |a();
     1304    207   
     1305    208    if (String.raw`test`)
     1306    209        a();
     1307
     1308INSERTING AT: 206:5
     1309PAUSES AT: 208:4
     1310    203        a();
     1311    204   
     1312    205    if (o1["p1"])
     1313 -> 206        a#();
     1314    207   
     1315 => 208    if (|String.raw`test`)
     1316    209        a();
     1317    210   
     1318
     1319INSERTING AT: 208:5
     1320PAUSES AT: 209:4
     1321    205    if (o1["p1"])
     1322    206        a();
     1323    207   
     1324 -> 208    if (S#tring.raw`test`)
     1325 => 209        |a();
     1326    210   
    12591327
    12601328

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

    r206653 r207312  
    29272927 => 200    |a(a(), b());
    29282928    201   
     2929    202    if (o1.p1)
     2930    203        a();
    29292931
    29302932
    … …  
    29362938-=> 200    |a(a(), b());
    29372939    201   
     2940    202    if (o1.p1)
     2941    203        a();
    29382942
    29392943
    29402944INSERTING AT: 201:0
     2945PAUSES AT: 202:4
     2946    198    var t2 = a`${1} ${x=1} ${a()}`;
     2947    199   
     2948    200    a(a(), b());
     2949 -> 201    #
     2950 => 202    if (|o1.p1)
     2951    203        a();
     2952    204   
     2953    205    if (o1["p1"])
     2954
     2955
     2956INSERTING AT: 202:0
     2957PAUSES AT: 202:4
     2958    199   
     2959    200    a(a(), b());
     2960    201   
     2961-=> 202    #if (|o1.p1)
     2962    203        a();
     2963    204   
     2964    205    if (o1["p1"])
     2965
     2966
     2967INSERTING AT: 203:0
     2968PAUSES AT: 203:4
     2969    200    a(a(), b());
     2970    201   
     2971    202    if (o1.p1)
     2972-=> 203    #    |a();
     2973    204   
     2974    205    if (o1["p1"])
     2975    206        a();
     2976
     2977
     2978INSERTING AT: 204:0
     2979PAUSES AT: 205:4
     2980    201   
     2981    202    if (o1.p1)
     2982    203        a();
     2983 -> 204    #
     2984 => 205    if (|o1["p1"])
     2985    206        a();
     2986    207   
     2987    208    if (String.raw`test`)
     2988
     2989
     2990INSERTING AT: 205:0
     2991PAUSES AT: 205:4
     2992    202    if (o1.p1)
     2993    203        a();
     2994    204   
     2995-=> 205    #if (|o1["p1"])
     2996    206        a();
     2997    207   
     2998    208    if (String.raw`test`)
     2999
     3000
     3001INSERTING AT: 206:0
     3002PAUSES AT: 206:4
     3003    203        a();
     3004    204   
     3005    205    if (o1["p1"])
     3006-=> 206    #    |a();
     3007    207   
     3008    208    if (String.raw`test`)
     3009    209        a();
     3010
     3011
     3012INSERTING AT: 207:0
     3013PAUSES AT: 208:4
     3014    204   
     3015    205    if (o1["p1"])
     3016    206        a();
     3017 -> 207    #
     3018 => 208    if (|String.raw`test`)
     3019    209        a();
     3020    210   
     3021
     3022
     3023INSERTING AT: 208:0
     3024PAUSES AT: 208:4
     3025    205    if (o1["p1"])
     3026    206        a();
     3027    207   
     3028-=> 208    #if (|String.raw`test`)
     3029    209        a();
     3030    210   
     3031
     3032
     3033INSERTING AT: 209:0
     3034PAUSES AT: 209:4
     3035    206        a();
     3036    207   
     3037    208    if (String.raw`test`)
     3038-=> 209    #    |a();
     3039    210   
     3040
     3041
     3042INSERTING AT: 210:0
    29413043PRODUCES: Could not resolve breakpoint
    29423044

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

    r206653 r207312  
    200200
    201201a(a(), b());
     202
     203if (o1.p1)
     204    a();
     205
     206if (o1["p1"])
     207    a();
     208
     209if (String.raw`test`)
     210    a();

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

    r206654 r207312  
    476476     72    }
    477477     73   
    478      74   
     478     74    function entryIfWithDotExpression() {
    479479
    480480ACTION: step-in
    … …  
    485485 ->  72    }|
    486486     73   
    487      74   
    488      75    // FIXME: Not Yet Tested
    489 
    490 ACTION: resume
    491 RESUMED
    492 PASS: Should have used all steps.
    493 
     487     74    function entryIfWithDotExpression() {
     488     75        var o = {condition: true};
     489
     490ACTION: resume
     491RESUMED
     492PASS: Should have used all steps.
     493
     494-- Running test case: Debugger.stepping.IfWithDotExpression
     495EXPRESSION: setTimeout(entryIfWithDotExpression)
     496STEPS: over, in, over, resume
     497PAUSED (debugger-statement)
     498PAUSE AT entryIfWithDotExpression:77:5
     499     73   
     500     74    function entryIfWithDotExpression() {
     501     75        var o = {condition: true};
     502 ->  76        |debugger;
     503     77        if (o.condition)
     504     78            a();
     505     79    }
     506
     507ACTION: step-over
     508PAUSE AT entryIfWithDotExpression:78:9
     509     74    function entryIfWithDotExpression() {
     510     75        var o = {condition: true};
     511     76        debugger;
     512 ->  77        if (|o.condition)
     513     78            a();
     514     79    }
     515     80   
     516
     517ACTION: step-in
     518PAUSE AT entryIfWithDotExpression:79:9
     519     75        var o = {condition: true};
     520     76        debugger;
     521     77        if (o.condition)
     522 ->  78            |a();
     523     79    }
     524     80   
     525     81    function entryIfWithBracketExpression() {
     526
     527ACTION: step-over
     528PAUSE AT entryIfWithDotExpression:80:2
     529     76        debugger;
     530     77        if (o.condition)
     531     78            a();
     532 ->  79    }|
     533     80   
     534     81    function entryIfWithBracketExpression() {
     535     82        var o = {condition: true};
     536
     537ACTION: resume
     538RESUMED
     539PASS: Should have used all steps.
     540
     541-- Running test case: Debugger.stepping.IfWithBracketExpression
     542EXPRESSION: setTimeout(entryIfWithBracketExpression)
     543STEPS: over, in, over, resume
     544PAUSED (debugger-statement)
     545PAUSE AT entryIfWithBracketExpression:84:5
     546     80   
     547     81    function entryIfWithBracketExpression() {
     548     82        var o = {condition: true};
     549 ->  83        |debugger;
     550     84        if (o["condition"])
     551     85            a();
     552     86    }
     553
     554ACTION: step-over
     555PAUSE AT entryIfWithBracketExpression:85:9
     556     81    function entryIfWithBracketExpression() {
     557     82        var o = {condition: true};
     558     83        debugger;
     559 ->  84        if (|o["condition"])
     560     85            a();
     561     86    }
     562     87   
     563
     564ACTION: step-in
     565PAUSE AT entryIfWithBracketExpression:86:9
     566     82        var o = {condition: true};
     567     83        debugger;
     568     84        if (o["condition"])
     569 ->  85            |a();
     570     86    }
     571     87   
     572     88    function entryIfWithTaggedTemplate() {
     573
     574ACTION: step-over
     575PAUSE AT entryIfWithBracketExpression:87:2
     576     83        debugger;
     577     84        if (o["condition"])
     578     85            a();
     579 ->  86    }|
     580     87   
     581     88    function entryIfWithTaggedTemplate() {
     582     89        debugger;
     583
     584ACTION: resume
     585RESUMED
     586PASS: Should have used all steps.
     587
     588-- Running test case: Debugger.stepping.IfWithTaggedTemplate
     589EXPRESSION: setTimeout(entryIfWithTaggedTemplate)
     590STEPS: over, in, over, resume
     591PAUSED (debugger-statement)
     592PAUSE AT entryIfWithTaggedTemplate:90:5
     593     86    }
     594     87   
     595     88    function entryIfWithTaggedTemplate() {
     596 ->  89        |debugger;
     597     90        if (String.raw`test`)
     598     91            a();
     599     92    }
     600
     601ACTION: step-over
     602PAUSE AT entryIfWithTaggedTemplate:91:9
     603     87   
     604     88    function entryIfWithTaggedTemplate() {
     605     89        debugger;
     606 ->  90        if (|String.raw`test`)
     607     91            a();
     608     92    }
     609     93   
     610
     611ACTION: step-in
     612PAUSE AT entryIfWithTaggedTemplate:92:9
     613     88    function entryIfWithTaggedTemplate() {
     614     89        debugger;
     615     90        if (String.raw`test`)
     616 ->  91            |a();
     617     92    }
     618     93   
     619     94   
     620
     621ACTION: step-over
     622PAUSE AT entryIfWithTaggedTemplate:93:2
     623     89        debugger;
     624     90        if (String.raw`test`)
     625     91            a();
     626 ->  92    }|
     627     93   
     628     94   
     629     95    // FIXME: Not Yet Tested
     630
     631ACTION: resume
     632RESUMED
     633PASS: Should have used all steps.
     634

  • trunk/LayoutTests/inspector/debugger/stepping/stepping-misc.html

    r206652 r207312  
    7171    var {x, y} = o;
    7272    var [w, z] = arr;
     73}
     74
     75function entryIfWithDotExpression() {
     76    var o = {condition: true};
     77    debugger;
     78    if (o.condition)
     79        a();
     80}
     81
     82function entryIfWithBracketExpression() {
     83    var o = {condition: true};
     84    debugger;
     85    if (o["condition"])
     86        a();
     87}
     88
     89function entryIfWithTaggedTemplate() {
     90    debugger;
     91    if (String.raw`test`)
     92        a();
    7393}
    7494
    … …  
    189209    });
    190210
     211    addSteppingTestCase({
     212        name: "Debugger.stepping.IfWithDotExpression",
     213        description: "Should step to the entire expression, not the dot.",
     214        expression: "setTimeout(entryIfWithDotExpression)",
     215        steps: [
     216            "over",
     217                "in",    // o.condition
     218                "over",  // a
     219            "resume",
     220        ]
     221    });
     222
     223    addSteppingTestCase({
     224        name: "Debugger.stepping.IfWithBracketExpression",
     225        description: "Should step to the entire expression, not the bracket dot.",
     226        expression: "setTimeout(entryIfWithBracketExpression)",
     227        steps: [
     228            "over",
     229                "in",    // o["condition"]
     230                "over",  // a
     231            "resume",
     232        ]
     233    });
     234
     235    addSteppingTestCase({
     236        name: "Debugger.stepping.IfWithTaggedTemplate",
     237        description: "Should step to the entire expression, not the template.",
     238        expression: "setTimeout(entryIfWithTaggedTemplate)",
     239        steps: [
     240            "over",
     241                "in",    // String.raw`test`
     242                "over",  // a
     243            "resume",
     244        ]
     245    });
     246
    191247    loadMainPageContent().then(() => {
    192248        suite.runTestCasesAndFinish();

  • trunk/Source/JavaScriptCore/ChangeLog

    r207289 r207312  
     12016-10-13  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Web Inspector: Stepping highlight for dot/bracket expressions in if statements highlights subset of the expression
     4        https://bugs.webkit.org/show_bug.cgi?id=163378
     5        <rdar://problem/28749376>
     6
     7        Reviewed by Saam Barati.
     8
     9        * parser/Parser.cpp:
     10        (JSC::Parser<LexerType>::parseAssignmentExpression):
     11        Since each expression builds on the previous, always keep the starting
     12        location the first location.
     13
    1142016-10-13  Per Arne Vollan  <pvollan@apple.com>
    215

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

    r207228 r207312  
    44854485            TreeExpression property = parseExpression(context);
    44864486            failIfFalse(property, "Cannot parse subscript expression");
    4487             base = context.createBracketAccess(location, base, property, initialAssignments != m_parserState.assignmentCount, expressionStart, expressionEnd, tokenEndPosition());
     4487            base = context.createBracketAccess(startLocation, base, property, initialAssignments != m_parserState.assignmentCount, expressionStart, expressionEnd, tokenEndPosition());
    44884488           
    44894489            if (UNLIKELY(baseIsSuper && currentScope()->isArrowFunction()))
    … …  
    45434543            nextExpectIdentifier(LexerFlagsIgnoreReservedWords | TreeBuilder::DontBuildKeywords);
    45444544            matchOrFail(IDENT, "Expected a property name after '.'");
    4545             base = context.createDotAccess(location, base, m_token.m_data.ident, expressionStart, expressionEnd, tokenEndPosition());
     4545            base = context.createDotAccess(startLocation, base, m_token.m_data.ident, expressionStart, expressionEnd, tokenEndPosition());
    45464546            if (UNLIKELY(baseIsSuper && currentScope()->isArrowFunction()))
    45474547                currentFunctionScope()->setInnerArrowFunctionUsesSuperProperty();
    … …  
    45554555            typename TreeBuilder::TemplateLiteral templateLiteral = parseTemplateLiteral(context, LexerType::RawStringsBuildMode::BuildRawStrings);
    45564556            failIfFalse(templateLiteral, "Cannot parse template literal");
    4557             base = context.createTaggedTemplate(location, base, templateLiteral, expressionStart, expressionEnd, lastTokenEndPosition());
     4557            base = context.createTaggedTemplate(startLocation, base, templateLiteral, expressionStart, expressionEnd, lastTokenEndPosition());
    45584558            m_parserState.nonLHSCount = nonLHSCount;
    45594559            break;
Note: See TracChangeset for help on using the changeset viewer.