1. 10 Jan, 2022 1 commit
    • Benedikt Meurer's avatar
      [inspector] Capture stack trace only once for JSError objects. · 765ca6a0
      Benedikt Meurer authored
      When creating a new JSError object (or using the non-standard API
      `Error.captureStackTrace`) V8 would previously capture the "simple stack
      trace" (as FixedArray of CallSiteInfo instances) to be used for the non-
      standard `error.stack` property, and if the inspector was active also
      capture the "detailed stack trace" (as FixedArray of StackFrameInfo
      instances). This turns out to be quite a lot of overhead, both in terms
      of execution time as well as memory pressure, especially since the
      information needed for the inspector is a proper subset of the
      information needed by `error.stack`.
      
      So this CL addresses the above issue by capturing only the "simple stack
      trace" (in the common case) and computing the "detailed stack trace"
      from the "simple stack trace" when on demand. This is accomplished by
      introducing a new ErrorStackData container that is used to store the
      stack trace information on JSErrors when the inspector is active. When
      capturing stack trace for a JSError object while the inspector is
      active, we take the maximum of the program controlled stack trace limit
      and the inspector requested stack trace limit, and memorize the program
      controlled stack trace limit for later formatting (to ensure that the
      presence of the inspector is not observable by the program).
      
      On the `standalone.js` benchmark from crbug.com/1283162 (with the
      default max call stack size of 200) we reduce execution time by around
      16% compared to ToT. And compared to V8 9.9.4 (the version prior to the
      regression in crbug.com/1280831), we are 6% faster now.
      
      Doc: https://bit.ly/v8-cheaper-inspector-stack-traces
      Bug: chromium:1280831, chromium:1278650, chromium:1258599
      Bug: chromium:1280803, chromium:1280832, chromium:1280818
      Fixed: chromium:1283162
      Change-Id: I57dac73e0ecf7d50ea57c3eb4981067deb28133e
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3366660Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#78542}
      765ca6a0
  2. 24 Aug, 2021 1 commit
    • Dan Elphick's avatar
      Reland "[include] Split out v8.h" · ec06bb6c
      Dan Elphick authored
      This is a reland of d1b27019
      
      Fixes include:
      Adding missing file to bazel build
      Forward-declaring classing before friend-classing them to fix win/gcc
      Add missing v8-isolate.h include for vtune builds
      
      Original change's description:
      > [include] Split out v8.h
      >
      > This moves every single class/function out of include/v8.h into a
      > separate header in include/, which v8.h then includes so that
      > externally nothing appears to have changed.
      >
      > Every include of v8.h from inside v8 has been changed to a more
      > fine-grained include.
      >
      > Previously inline functions defined at the bottom of v8.h would call
      > private non-inline functions in the V8 class. Since that class is now
      > in v8-initialization.h and is rarely included (as that would create
      > dependency cycles), this is not possible and so those methods have been
      > moved out of the V8 class into the namespace v8::api_internal.
      >
      > None of the previous files in include/ now #include v8.h, which means
      > if embedders were relying on this transitive dependency then it will
      > give compile failures.
      >
      > v8-inspector.h does depend on v8-scripts.h for the time being to ensure
      > that Chrome continue to compile but that change will be reverted once
      > those transitive #includes in chrome are changed to include it directly.
      >
      > Full design:
      > https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing
      >
      > Bug: v8:11965
      > Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448
      > Reviewed-by: Yang Guo <yangguo@chromium.org>
      > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
      > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
      > Reviewed-by: Leszek Swirski <leszeks@chromium.org>
      > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
      > Commit-Queue: Dan Elphick <delphick@chromium.org>
      > Cr-Commit-Position: refs/heads/main@{#76424}
      
      Cq-Include-Trybots: luci.v8.try:v8_linux_vtunejit
      Bug: v8:11965
      Change-Id: I99f5d3a73bf8fe25b650adfaf9567dc4e44a09e6
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3113629Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
      Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
      Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Commit-Queue: Dan Elphick <delphick@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#76460}
      ec06bb6c
  3. 23 Aug, 2021 2 commits
    • Dan Elphick's avatar
      Revert "[include] Split out v8.h" · 44fe02ce
      Dan Elphick authored
      This reverts commit d1b27019.
      
      Reason for revert: Broke vtune build, tsan build and possibly others
      
      Original change's description:
      > [include] Split out v8.h
      >
      > This moves every single class/function out of include/v8.h into a
      > separate header in include/, which v8.h then includes so that
      > externally nothing appears to have changed.
      >
      > Every include of v8.h from inside v8 has been changed to a more
      > fine-grained include.
      >
      > Previously inline functions defined at the bottom of v8.h would call
      > private non-inline functions in the V8 class. Since that class is now
      > in v8-initialization.h and is rarely included (as that would create
      > dependency cycles), this is not possible and so those methods have been
      > moved out of the V8 class into the namespace v8::api_internal.
      >
      > None of the previous files in include/ now #include v8.h, which means
      > if embedders were relying on this transitive dependency then it will
      > give compile failures.
      >
      > v8-inspector.h does depend on v8-scripts.h for the time being to ensure
      > that Chrome continue to compile but that change will be reverted once
      > those transitive #includes in chrome are changed to include it directly.
      >
      > Full design:
      > https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing
      >
      > Bug: v8:11965
      > Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448
      > Reviewed-by: Yang Guo <yangguo@chromium.org>
      > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
      > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
      > Reviewed-by: Leszek Swirski <leszeks@chromium.org>
      > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
      > Commit-Queue: Dan Elphick <delphick@chromium.org>
      > Cr-Commit-Position: refs/heads/main@{#76424}
      
      Bug: v8:11965
      Change-Id: Id57313ae992e720c8b19abc975cd69729e1344aa
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3113627
      Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
      Commit-Queue: Leszek Swirski <leszeks@chromium.org>
      Owners-Override: Leszek Swirski <leszeks@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#76428}
      44fe02ce
    • Dan Elphick's avatar
      [include] Split out v8.h · d1b27019
      Dan Elphick authored
      This moves every single class/function out of include/v8.h into a
      separate header in include/, which v8.h then includes so that
      externally nothing appears to have changed.
      
      Every include of v8.h from inside v8 has been changed to a more
      fine-grained include.
      
      Previously inline functions defined at the bottom of v8.h would call
      private non-inline functions in the V8 class. Since that class is now
      in v8-initialization.h and is rarely included (as that would create
      dependency cycles), this is not possible and so those methods have been
      moved out of the V8 class into the namespace v8::api_internal.
      
      None of the previous files in include/ now #include v8.h, which means
      if embedders were relying on this transitive dependency then it will
      give compile failures.
      
      v8-inspector.h does depend on v8-scripts.h for the time being to ensure
      that Chrome continue to compile but that change will be reverted once
      those transitive #includes in chrome are changed to include it directly.
      
      Full design:
      https://docs.google.com/document/d/1rTD--I8hCAr-Rho1WTumZzFKaDpEp0IJ8ejZtk4nJdA/edit?usp=sharing
      
      Bug: v8:11965
      Change-Id: I53b84b29581632710edc80eb11f819c2097a2877
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3097448Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarCamillo Bruni <cbruni@chromium.org>
      Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
      Commit-Queue: Dan Elphick <delphick@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#76424}
      d1b27019
  4. 27 May, 2021 1 commit
    • Manos Koukoutos's avatar
      [wasm-gc] Implement array.copy (experimental) · dfdc8f68
      Manos Koukoutos authored
      Changes:
      - Add --experimental-wasm-gc-experiments flag.
      - Add array.copy opcode. Implement it in decoding and code generation
        behind the new flag.
      - Add WasmCodeBuilder::BoundsCheckArrayCopy. Move BoundsCheckArray to
        the private section.
      - Add WasmArrayCopy and WasmArrayCopyWithChecks builtin.
      - Add WasmArrayCopy runtime function.
      - Add WasmArray::ElementSlot.
      - Always print two hex digits in CHECK_PROTOTYPE_OPCODE.
      - In test-gc, print the thrown-error message if the function should not
        throw.
      - In test-gc, add GetResultObject with one argument.
      
      Bug: v8:7748
      Change-Id: I58f4d37e254154596cdef5e78482b55260dd3782
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2912729
      Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
      Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#74806}
      dfdc8f68
  5. 08 Apr, 2021 1 commit
  6. 12 Feb, 2021 1 commit
    • Benedikt Meurer's avatar
      [stack-traces] Simplify and speedup stack trace collection. · eed0d27c
      Benedikt Meurer authored
      Following up on https://crrev.com/c/2689185, this CL significantly
      simplifies the whole implementation of the stack trace capturing.
      
      Before this CL, capturing any stack trace (for the purpose of the API or
      Error.stack) would roughly work like this:
      
        1. The CaptureStackTrace() function uses the StackFrameIterator to
           walk the system stack. For each native frame it uses the
           FrameSummary abstraction to get all (including potentially inlined)
           frames. For each of those it appends a record consisting of six
           elements to a FrameArray (this holds pointers to the actual
           closures and receivers).
        2. Afterwards the FrameArray is shrinked to the required size, and a
           new FixedArray is allocated, and initialized with new
           StackTraceFrame objects where each holds a reference to the
           FrameArray, the index of the frame, and an initially uninitialized
           StackFrameInfo reference. This new FixedArray is then returned from
           CaptureStackTrace() and either stored on a message object or
           provided to the API as v8::StackTrace.
      
      The new approach removes a lot of the machinery in between and directly
      creates a FixedArray of StackFrameInfo objects in CaptureStackTrace().
      These StackFrameInfo objects are directly exposed as v8::StackFrame on
      the public API, and they hold the six fields that were previously stored
      flat in the FrameArray. This not only avoids a lot of copying around of
      data and creation of temporary objects and handles, but most importantly
      unifies and simplifies the stack frame function inside StackFrameInfo,
      so you no longer need to wonder which function / object might be
      responsible for a certain API.
      
      There's still a lot of room for improvement. In particular we currently
      don't cache the source position for a given StackFrameInfo (or
      globally), but rather recompute it every time. This is still very fast,
      significantly faster than the previous approach.
      
      There are some notable (potentially user visible) changes:
      
        - The CallSite#GetPosition() method now consistently returns the
          Wasm module relative bytecode offset for all Wasm frames (previously
          it'd return the function relative bytecode offset for non-asm.js
          Wasm frames).
        - The column and line numbers returned from StackFrameInfo methods are
          consistently 1-based now, instead of sometimes being 0-based (Wasm)
          and sometimes being 1-based (JS and asm.js Wasm). The only
          potentially noticable difference is that for
          CallSite#GetLineNumber() no longer returns 0 for Wasm frames, but
          that was wrong and useless anyways.
        - CallSite#GetThis() would sometimes return the_hole, another bug
          flushed out by this CL.
      
      The CL also contains some other not noteworthy drive-by-cleanups.
      
      Fixed: chromium:1057211
      Bug: chromium:1077657, chromium:1069425, v8:8742
      Bug: chromium:1127391, chromium:1098530, chromium:981541
      Change-Id: Iff12f6838a4d99080db8dd96bccc14440affc5a5
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2689183
      Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#72694}
      eed0d27c
  7. 09 Feb, 2021 1 commit
  8. 23 Nov, 2020 3 commits
  9. 24 Oct, 2020 1 commit
    • Camillo Bruni's avatar
      Reland "[runtime] Use Isolate::ThrowAt with MessageLocation" · 447915ef
      Camillo Bruni authored
      This is a reland of eb6b4ce1
      
      Skip test that serializes Error which references a Script. All errors
      created by ThrowAt store the current Script under the
      error_script_symbol.
      
      Original change's description:
      > [runtime] Use Isolate::ThrowAt with MessageLocation
      >
      > Fix various missing source positions when reporting parse and compile
      > errors. Namely this fixes missing source positions when having invalid
      > module imports.
      >
      > - Use Isolate::ThrowAt with valid MessageLocation objects
      > - Change public Isolate::Throw to no longer accept MessageLocation to
      >   avoid misues
      > - Introduce private Isolate::ThrowInternal that accepts MessageLocation
      >
      > Bug: v8:6513
      > Change-Id: I3ee633c9fff8c9d361bddb37f56e28a50c280ec1
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2467839
      > Commit-Queue: Camillo Bruni <cbruni@chromium.org>
      > Reviewed-by: Marja Hölttä <marja@chromium.org>
      > Reviewed-by: Igor Sheludko <ishell@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#70623}
      
      Bug: v8:6513
      Change-Id: Icba74f74178e28fbda0fd0c237eeb7bacbc33570
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2487123Reviewed-by: 's avatarIgor Sheludko <ishell@chromium.org>
      Reviewed-by: 's avatarMarja Hölttä <marja@chromium.org>
      Commit-Queue: Camillo Bruni <cbruni@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#70741}
      447915ef
  10. 19 Oct, 2020 2 commits
  11. 17 Aug, 2020 1 commit
  12. 24 Jun, 2020 1 commit
    • Clemens Backes's avatar
      [wasm] Make opcode properties constexpr · 852f43cd
      Clemens Backes authored
      This allows the compiler to eliminate more unneeded branches. Since all
      functions just do a lookup in a static table (either directly, or via
      compiling a switch to such a lookup), they are also good candidates for
      inlining, which is made possible by this change.
      
      One DCHECK is removed instead of pulling in the inl header, which would
      require more refactoring since the check is in a non-inl header.
      
      R=thibaudm@chromium.org
      TBR=jkummerow@chromium.org
      
      Bug: v8:10576
      Change-Id: If0fd25fd62c5f30b896fc67a5458a5ae475a6351
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2259944
      Commit-Queue: Clemens Backes <clemensb@chromium.org>
      Reviewed-by: 's avatarThibaud Michaud <thibaudm@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#68508}
      852f43cd
  13. 29 May, 2020 1 commit
  14. 26 May, 2020 1 commit
    • Seth Brenith's avatar
      Revert "[torque][cleanup] Use more precise field types in a few classes" · 16cb2d94
      Seth Brenith authored
      This reverts commit 4e5fabae.
      
      Reason for revert: performance regressions chromium:1085305, chromium:1084978
      
      Original change's description:
      > [torque][cleanup] Use more precise field types in a few classes
      > 
      > This change updates some Torque-defined classes to include more precise
      > field types where possible. It also updates those classes to use
      > @generateCppClass. One field was removed because it's unused
      > (PrototypeInfo::validity_cell), and two fields in StackFrameInfo
      > actually became less precise because they're based on Script::name,
      > which is an embedder-provided untyped Local<Value>. (Automatically
      > generated accessors pointed out this bug easily.)
      > 
      > This change also includes a couple of minor fixes in Torque.
      > 
      > Change-Id: Ib2bc6c7165bb3612b6d344c0686a94165a568277
      > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2199640
      > Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
      > Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
      > Reviewed-by: Toon Verwaest <verwaest@chromium.org>
      > Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
      > Cr-Commit-Position: refs/heads/master@{#67907}
      
      TBR=ulan@chromium.org,tebbi@chromium.org,verwaest@chromium.org,seth.brenith@microsoft.com
      
      Change-Id: I720821d8dc84ea0d79eb137f1c2507f75df9a107
      No-Presubmit: true
      No-Tree-Checks: true
      No-Try: true
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2211322Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
      Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#67972}
      16cb2d94
  15. 19 May, 2020 1 commit
  16. 13 May, 2020 1 commit
  17. 30 Apr, 2020 1 commit
  18. 24 Apr, 2020 1 commit
  19. 20 Apr, 2020 1 commit
  20. 18 Feb, 2020 1 commit
  21. 05 Feb, 2020 1 commit
    • Sathya Gunasekaran's avatar
      [callprinter] Correctly point to the incorrect spread arg · 1d0693e2
      Sathya Gunasekaran authored
      The source position is set to the function call (console.log) not the
      spread (..x), in the bytecode generator, as the spread operation is
      done as part of the CallWithSpread bytecode.
      
      The CallPrinter stops at the function call and doesn't look at the
      arguments as well (in CallPrinter::VisitCall) to see if the error is
      from an incorrect spread operation.
      
      
      With this patch, we pass some state to the CallPrinter in the
      CallWithSpread error case and check that in CallPrinter::VisitCall
      before returning.
      
      For the given source string:
      ```
      x = undefined;
      console.log(1, ...x);
      ```
      
      Previously, the error was -
      
      ```
      test.js:2: TypeError: console.log is not iterable (cannot read property Symbol(Symbol.iterator))
      console.log(1, ...x);
              ^
      TypeError: console.log is not iterable (cannot read property Symbol(Symbol.iterator))
          at test.js:2:9
      ```
      
      
      Now, the error is -
      
      ```
      _test.js:2: TypeError: x is not iterable (cannot read property undefined)
      console.log(1, ...x);
                        ^
      TypeError: x is not iterable (cannot read property undefined)
          at _test.js:2:9
      ```
      
      Bug: v8:10038
      Change-Id: I199de9997f1d949c6f9b7b4f41d51f422b8b5131
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2037431Reviewed-by: 's avatarJakob Kummerow <jkummerow@chromium.org>
      Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
      Commit-Queue: Sathya Gunasekaran  <gsathya@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#66131}
      1d0693e2
  22. 22 Jan, 2020 2 commits
  23. 16 Jan, 2020 1 commit
  24. 05 Nov, 2019 1 commit
    • Eric Leese's avatar
      V8 Wasm locations should always be based on byte offsets · 5c23e6b5
      Eric Leese authored
      Currently there are two ways wasm locations are represented in the
      inspector. This remains unchanged for now. Also, currently there are
      multiple ways location is represented within V8, with the line number
      sometimes being a function index and sometimes being 0, and the column
      number being a byte offset which is sometimes function relative and
      sometimes module relative. With this change, the line number is never
      used within V8 (it is always 0), and the column number is always a
      byte offset from the beginning of the module. This simplifies
      translation logic and keeps it in one place, and will simplify future
      changes to wasm location representation in the inspector API.
      
      Bug: chromium:1013527
      Change-Id: I8813d47c881988f9ab49d7529fb81fe10dbbccff
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1886915
      Commit-Queue: Eric Leese <leese@chromium.org>
      Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
      Reviewed-by: 's avatarYang Guo <yangguo@chromium.org>
      Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#64774}
      5c23e6b5
  25. 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
  26. 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
  27. 08 Aug, 2019 1 commit
  28. 24 Jul, 2019 1 commit
  29. 08 Jul, 2019 1 commit
  30. 04 Jul, 2019 1 commit
    • Simon Zünd's avatar
      [stack-trace] Separate stack-trace symbolization and serialization · db24e200
      Simon Zünd authored
      This CL moves the code responsible for serializing a stack trace frame into
      a string, out of messages.cc and into stack-frame-info.cc. Instead of
      symbolizing the stack trace frame while serializing, the code is changed to
      work on top of StackTraceFrame and StackFrameInfo objects.
      
      The result is that the serialization code no longer cares when a stack trace
      frame is symbolized. Symbolization could happen eagerly during capturing, or
      lazily the first time any of StackFrameInfo fields are accessed.
      
      Drive-by: Existing users of StackFrameBase::ToString are adapted to the
      new SerializeStackTraceFrame API. This includes Isolate::PrintCurrentStackTrace,
      which is changed to re-use the existing capturing and serializing mechanism.
      
      Bug: v8:8742
      Change-Id: Ic7fd80668c9d993e99d586ef7fe022850104c34f
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1631414
      Commit-Queue: Simon Zünd <szuend@chromium.org>
      Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
      Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#62522}
      db24e200
  31. 26 Jun, 2019 1 commit
  32. 14 Jun, 2019 1 commit
    • Simon Zünd's avatar
      [stack-trace] Change column number for wasm frames to module offset · f16f0bcc
      Simon Zünd authored
      The CL https://crrev.com/c/1646846 changed column numbers for Wasm
      frames in Error.stack traces. Instead of using the offset relative to
      the beginning of the function, the absolute offset inside the module
      is displayed as hex.
      
      This CL propagates that change to the StackTrace C++ API, so
      StackFrame::GetColumn() also returns the absolute offset. Note that the
      StackFrame API historically uses "0" to signal "no information", so the
      line and column numbers for Wasm frames are also adjusted to 1-based,
      even though they signify function index and absolute offset
      into the module.
      
      This CL does not touch Script::PositionInfo.column. That field still
      contains the offset relative to the function start.
      
      Bug: v8:8742
      Change-Id: If4fd37fa681c7ebd0823ce0d95eccc1335c35272
      Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1655300
      Commit-Queue: Simon Zünd <szuend@chromium.org>
      Reviewed-by: 's avatarBen Titzer <titzer@chromium.org>
      Reviewed-by: 's avatarMichael Lippautz <mlippautz@chromium.org>
      Cr-Commit-Position: refs/heads/master@{#62171}
      f16f0bcc
  33. 23 May, 2019 2 commits