1. 30 Aug, 2019 1 commit
    • Leszek Swirski's avatar
      Reland "[destructuring] Elide coercible check for simple keys" · ef2df57a
      Leszek Swirski authored
      This is a reland of 1fba0441
      Chromium expectation tests have been disabled, and will be enabled
      
      Original change's description:
      > [destructuring] Elide coercible check for simple keys
      >
      > Simple object destructuring, such as `let {a,b} = o`, is less efficient
      > than the equivalent assignments `let a = o.a; let b = o.b`. This is
      > because it does a nil check of `o` before the assignments. However, this
      > nil check is not strictly necessary for simple (i.e. non-computed) names,
      > as there will be an equivalent nil check on the first access to o in
      > `o.a`. For computed names the computation is unfortunately obervable.
      >
      > So, we can elide the nil check when the first property (if any) of the
      > destructuring target is a non-computed name. This messes a bit with our
      > error messages, so we re-use the CallPrinter to also find destructuring
      > assignment based errors, and fiddle with the error message there. As
      > a side-effect, we also get out the object name in the AST, so we can
      > output a slightly nicer error message.
      >
      > Change-Id: Iafa858e27ed771a146cd3ba57903cc73bb46951d
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1773254
      > Reviewed-by: Leszek Swirski <leszeks@chromium.org>
      > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#63453}
      
      TBR=verwaest@chromium.org
      
      Bug: chromium:999473
      Change-Id: Ib0b2e4be433c50521ba1722e1c06b672bfefa405
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1777702Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#63477}
      ef2df57a
  2. 29 Aug, 2019 2 commits
    • Adam Klein's avatar
      Revert "[destructuring] Elide coercible check for simple keys" · 28fa4cb4
      Adam Klein authored
      This reverts commit 1fba0441.
      
      Reason for revert: blocks V8 roll due to layout test failures caused by error message changes:
      https://ci.chromium.org/p/v8/builders/ci/V8%20Blink%20Linux/347
      
      Original change's description:
      > [destructuring] Elide coercible check for simple keys
      > 
      > Simple object destructuring, such as `let {a,b} = o`, is less efficient
      > than the equivalent assignments `let a = o.a; let b = o.b`. This is
      > because it does a nil check of `o` before the assignments. However, this
      > nil check is not strictly necessary for simple (i.e. non-computed) names,
      > as there will be an equivalent nil check on the first access to o in
      > `o.a`. For computed names the computation is unfortunately obervable.
      > 
      > So, we can elide the nil check when the first property (if any) of the
      > destructuring target is a non-computed name. This messes a bit with our
      > error messages, so we re-use the CallPrinter to also find destructuring
      > assignment based errors, and fiddle with the error message there. As
      > a side-effect, we also get out the object name in the AST, so we can
      > output a slightly nicer error message.
      > 
      > Change-Id: Iafa858e27ed771a146cd3ba57903cc73bb46951d
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1773254
      > Reviewed-by: Leszek Swirski <leszeks@chromium.org>
      > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#63453}
      
      TBR=leszeks@chromium.org,verwaest@chromium.org
      
      Change-Id: I74cf06ebd987e5b8bbe1831b0042c085edf37f5b
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1776994Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
      Commit-Queue: Adam Klein <adamk@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#63465}
      28fa4cb4
    • Leszek Swirski's avatar
      [destructuring] Elide coercible check for simple keys · 1fba0441
      Leszek Swirski authored
      Simple object destructuring, such as `let {a,b} = o`, is less efficient
      than the equivalent assignments `let a = o.a; let b = o.b`. This is
      because it does a nil check of `o` before the assignments. However, this
      nil check is not strictly necessary for simple (i.e. non-computed) names,
      as there will be an equivalent nil check on the first access to o in
      `o.a`. For computed names the computation is unfortunately obervable.
      
      So, we can elide the nil check when the first property (if any) of the
      destructuring target is a non-computed name. This messes a bit with our
      error messages, so we re-use the CallPrinter to also find destructuring
      assignment based errors, and fiddle with the error message there. As
      a side-effect, we also get out the object name in the AST, so we can
      output a slightly nicer error message.
      
      Change-Id: Iafa858e27ed771a146cd3ba57903cc73bb46951d
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1773254Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#63453}
      1fba0441
  3. 09 Jan, 2019 1 commit
    • Leszek Swirski's avatar
      [parser] Don't desugar destructuring declarations. · 5e725a2b
      Leszek Swirski authored
      Emit a single destructuring assignment for destructuring declarations,
      which can be desugared by the bytecode generator. This allows us to
      remove destructuring desugaring from the parser (specifically, the
      pattern rewriter) entirely.
      
      The pattern "rewriter" is now only responsible for walking the
      destructuring pattern to declare variables, mark them assigned, and
      potentially rewrite scopes for the edge case of parameters with a sloppy
      eval.
      
      Note that since the rewriter is no longer rewriting, we have to flip the
      VariableProxy copying logic for var re-lookup, so that we now pass the
      new VariableProxy to the variable declaration and leave the original
      unresolved (rather than passing the original through and rewriting to a
      new unresolved VariableProxy).
      
      This change does have some effect on breakpoint locations, due to some
      of the available information changing between the parser and bytecode
      generator, however the new locations appear to be more consistent
      between assignments and declarations.
      
      Change-Id: I3a58dd0a387d2bfb8e5e9e22dde0acc5f440cb82
      Reviewed-on: https://chromium-review.googlesource.com/c/1382462
      Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
      Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarToon Verwaest <verwaest@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#58670}
      5e725a2b
  4. 26 Oct, 2017 1 commit
  5. 15 Jun, 2017 1 commit
    • Sathya Gunasekaran's avatar
      [parser] Better error message when destructuring against undefined/null · bc2c785c
      Sathya Gunasekaran authored
      Previously, when destructuring against null or undefined we would
      print:
      
        d8> var { x } = null
        (d8):1: TypeError: Cannot match against 'undefined' or 'null'.
        var { x } = null
        ^
        TypeError: Cannot match against 'undefined' or 'null'.
            at (d8):1:1
      
      
      The above message uses the term "match" which isn't a common term in
      JavaScript to describe destructuring. This message also doesn't
      provide the name of the property that fails destructuring.
      
      This patch changes the error message to be:
      
        d8> var { x } = null;
        (d8):1: TypeError: Cannot destructure property `x` of 'undefined' or 'null'.
        var { x } = null;
              ^
        TypeError: Cannot destructure property `x` of 'undefined' or 'null'.
            at (d8):1:1
      
      This patch changes the message to say "destructure" instead of "match".
      
      This patch adds support for printing property names that are string
      literals. We iterate through every property and pick the first string
      literal property name if it exists. This provides at least some
      feedback to the developer.
      
      This patch also makes the pointer point to the position of the
      property name that fails destructuring.
      
      For computed and numeric property names, we print a generic error:
        d8> var { 1: x } = null
        (d8):1: TypeError: Cannot destructure against 'undefined' or 'null'.
        var { 1: x } = null
        ^
        TypeError: Cannot destructure against 'undefined' or 'null'.
            at (d8):1:1
      
      Bug: v8:6499
      Change-Id: I35b1ac749489828686f042975294b9926e2dfc53
      Reviewed-on: https://chromium-review.googlesource.com/537341Reviewed-by: 's avatarAdam Klein <adamk@chromium.org>
      Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#45965}
      bc2c785c