Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v8.x] deps: cherry-pick c3458a8 from upstream V8 #18059

Closed
wants to merge 1 commit into from
Closed

[v8.x] deps: cherry-pick c3458a8 from upstream V8 #18059

wants to merge 1 commit into from

Conversation

targos
Copy link
Member

@targos targos commented Jan 9, 2018

Original commit message:

[parser] Add new FunctionNameInferrer state before parsing param

Create new state before parsing FormalParameter because we don't
want to use any of the parameters as an inferred function name.

Previously the stacktrace was:
  test.js:3: Error: boom
      throw new Error('boom');
      ^
  Error: boom
      at param (test.js:3:11)
      at test.js:4:5
      at test.js:6:3

The stacktrace with this patch:
  test.js:3: Error: boom
      throw new Error('boom');
      ^
  Error: boom
      at test.js:3:11
      at test.js:4:5
      at test.js:6:3

Bug: v8:6822, v8:6513
Change-Id: Ifbadc660fc4e85248af405acd67c025f11662bd4
Reviewed-on: https://chromium-review.googlesource.com/742657
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49042}

Refs: v8/v8@c3458a8
Closes: #15386

/cc @nodejs/v8

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

V8

@targos
deps: cherry-pick c3458a8 from upstream V8
628c89b 
Original commit message:

    [parser] Add new FunctionNameInferrer state before parsing param

    Create new state before parsing FormalParameter because we don't
    want to use any of the parameters as an inferred function name.

    Previously the stacktrace was:
      test.js:3: Error: boom
          throw new Error('boom');
          ^
      Error: boom
          at param (test.js:3:11)
          at test.js:4:5
          at test.js:6:3

    The stacktrace with this patch:
      test.js:3: Error: boom
          throw new Error('boom');
          ^
      Error: boom
          at test.js:3:11
          at test.js:4:5
          at test.js:6:3

    Bug: v8:6822, v8:6513
    Change-Id: Ifbadc660fc4e85248af405acd67c025f11662bd4
    Reviewed-on: https://chromium-review.googlesource.com/742657
    Reviewed-by: Adam Klein <adamk@chromium.org>
    Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#49042}

Refs: v8/v8@c3458a8
Closes: nodejs#15386
@nodejs-github-bot nodejs-github-bot added v8 engine Issues and PRs related to the V8 dependency. v8.x labels Jan 9, 2018
@targos targos mentioned this pull request Jan 9, 2018
Closed
mhdawson
mhdawson approved these changes Jan 9, 2018
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@targos
Copy link
Member Author

targos commented Jan 11, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/12498/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/1155/

@targos
Copy link
Member Author

targos commented Jan 21, 2018

@nodejs/lts

@gibfahn gibfahn self-assigned this Jan 21, 2018
MylesBorins pushed a commit that referenced this pull request Feb 7, 2018
@targos @MylesBorins
deps: cherry-pick c3458a8 from upstream V8
3a648b7 
Original commit message:

    [parser] Add new FunctionNameInferrer state before parsing param

    Create new state before parsing FormalParameter because we don't
    want to use any of the parameters as an inferred function name.

    Previously the stacktrace was:
      test.js:3: Error: boom
          throw new Error('boom');
          ^
      Error: boom
          at param (test.js:3:11)
          at test.js:4:5
          at test.js:6:3

    The stacktrace with this patch:
      test.js:3: Error: boom
          throw new Error('boom');
          ^
      Error: boom
          at test.js:3:11
          at test.js:4:5
          at test.js:6:3

    Bug: v8:6822, v8:6513
    Change-Id: Ifbadc660fc4e85248af405acd67c025f11662bd4
    Reviewed-on: https://chromium-review.googlesource.com/742657
    Reviewed-by: Adam Klein <adamk@chromium.org>
    Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#49042}

PR-URL: #18059
Closes: #15386
Fixes: #15386
Refs: v8/v8@c3458a8
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@MylesBorins
Copy link
Contributor

landed in 3a648b7

@MylesBorins MylesBorins closed this Feb 7, 2018
@targos targos deleted the fix-15386-8.x branch October 17, 2020 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fhinkel fhinkel fhinkel approved these changes

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@cjihrig cjihrig cjihrig approved these changes

@mhdawson mhdawson mhdawson approved these changes

Assignees

@gibfahn gibfahn

Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@targos @MylesBorins @fhinkel @bnoordhuis @cjihrig @mhdawson @gibfahn @nodejs-github-bot