deps: V8: backport 4263f8a5e8e0 #35650

bdougie · 2020-10-14T21:57:46Z

Original commit message:

parser: better error message for await+tla

Bug: v8:9344, v8:6513
Change-Id: I1854e483515e7da99192367b6764a0ec7c8b41d9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2411687
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Gus Caplan <snek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70099}

Refs: v8/v8@4263f8a

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

bdougie · 2020-10-14T21:59:09Z

re: #35196 (comment)

I originally opened the above PR updating the error messages. That was instead landed upstream. This backport adds those changes and closes out #35196

nodejs-github-bot · 2020-10-14T22:02:14Z

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

MylesBorins

some tiny issues with conflict resolution

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden

nodejs-github-bot · 2020-10-14T22:27:01Z

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

MylesBorins · 2020-10-15T01:10:28Z

@devsnek any idea why the V8 tests are failing?

bdougie · 2020-10-15T19:49:11Z

The failures are in files that I touched (fixed merge conflicts). Curious if it is a white space thing or similar.

:53: Uncaught TypeError: '#b' was defined without a setter
:46: Uncaught TypeError: '#c' was defined without a getter
:61: Uncaught TypeError: '#d' was defined without a setter
:46: Uncaught TypeError: '#e' was defined without a getter
stderr:
Generated has extra lines after line 182
Generated: ''

MylesBorins · 2020-10-16T00:35:34Z

/cc @nodejs/v8-update

Trott · 2020-10-21T13:09:36Z

I believe V8-CI skips rebasing by default. I ran it again, but this time rebasing to see if that fixed things. https://ci.nodejs.org/job/node-test-commit-v8-linux/3493/ It reported that it couldn't rebase, which is a known issue in our CI with merge commits. I'll force-push out the merge commit from @MylesBorins and try again...

Trott · 2020-10-21T13:13:11Z

V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3494/

Trott

LGTM if we can get the V8 CI to pass

MylesBorins · 2020-10-21T14:19:31Z

@Trott the issue with CI is that the .golden files need to be regenerated and afaict there is no easy way for us to do this. @devsnek was looking into this, and I believe chatted with @joyeecheung about it to.

Maybe someone from @nodejs/v8-update would know how to help.

joyeecheung · 2020-10-21T15:45:29Z

The failures are in files that I touched (fixed merge conflicts). Curious if it is a white space thing or similar.

Yes you need to remove the whitespace changes.

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden

joyeecheung · 2020-10-21T15:49:32Z

Looks like the CI jobs are still failing because of git issues

stderr: error: you need to resolve your current index first

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden

Original commit message: parser: better error message for await+tla Bug: v8:9344, v8:6513 Change-Id: I1854e483515e7da99192367b6764a0ec7c8b41d9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2411687 Reviewed-by: Marja Hölttä <marja@chromium.org> Commit-Queue: Gus Caplan <snek@chromium.org> Cr-Commit-Position: refs/heads/master@{#70099} Refs: v8/v8@4263f8a

MylesBorins · 2020-10-21T16:16:37Z

@joyeecheung it looks like the updates to the golden files from the original CL got lost when we updated V8 on master. I've re-applied the changes

edit:

Another crack at V8-CI https://ci.nodejs.org/job/node-test-commit-v8-linux/3495/

joyeecheung · 2020-10-21T17:39:40Z

I did a rebaseline locally and this is the patch I get (compared to the state after this force push) and it passes the cctest locally. Should I update this branch?

See diff

diff --git a/deps/v8/test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden b/deps/v8/test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden
index 5ce6e397..6d5bb4a5 100644
--- a/deps/v8/test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden
+++ b/deps/v8/test/cctest/interpreter/bytecode_expectations/StaticPrivateMethodAccess.golden
@@ -25,7 +25,7 @@ bytecodes: [
                 B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(1),
                 B(JumpIfTrue), U8(18),
-                B(Wide), B(LdaSmi), I16(262),
+                B(Wide), B(LdaSmi), I16(263),
                 B(Star), R(2),
                 B(LdaConstant), U8(0),
                 B(Star), R(3),
@@ -56,7 +56,7 @@ frame size: 2
 parameter count: 1
 bytecode array length: 16
 bytecodes: [
-  /*   56 S> */ B(Wide), B(LdaSmi), I16(264),
+  /*   56 S> */ B(Wide), B(LdaSmi), I16(265),
                 B(Star), R(0),
                 B(LdaConstant), U8(0),
                 B(Star), R(1),
@@ -83,7 +83,7 @@ frame size: 2
 parameter count: 1
 bytecode array length: 16
 bytecodes: [
-  /*   56 S> */ B(Wide), B(LdaSmi), I16(264),
+  /*   56 S> */ B(Wide), B(LdaSmi), I16(265),
                 B(Star), R(0),
                 B(LdaConstant), U8(0),
                 B(Star), R(1),
@@ -122,7 +122,7 @@ bytecodes: [
   /*   94 E> */ B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(18),
-                B(Wide), B(LdaSmi), I16(262),
+                B(Wide), B(LdaSmi), I16(263),
                 B(Star), R(2),
                 B(LdaConstant), U8(0),
                 B(Star), R(3),
@@ -144,7 +144,7 @@ bytecodes: [
   /*  109 E> */ B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(1),
                 B(JumpIfTrue), U8(18),
-                B(Wide), B(LdaSmi), I16(263),
+                B(Wide), B(LdaSmi), I16(264),
                 B(Star), R(3),
                 B(LdaConstant), U8(0),
                 B(Star), R(4),
@@ -159,7 +159,7 @@ bytecodes: [
   /*  133 E> */ B(TestReferenceEqual), R(this),
                 B(Mov), R(this), R(0),
                 B(JumpIfTrue), U8(18),
-                B(Wide), B(LdaSmi), I16(262),
+                B(Wide), B(LdaSmi), I16(263),
                 B(Star), R(2),
                 B(LdaConstant), U8(0),
                 B(Star), R(3),
@@ -189,7 +189,7 @@ frame size: 2
 parameter count: 1
 bytecode array length: 16
 bytecodes: [
-  /*   60 S> */ B(Wide), B(LdaSmi), I16(266),
+  /*   60 S> */ B(Wide), B(LdaSmi), I16(267),
                 B(Star), R(0),
                 B(LdaConstant), U8(0),
                 B(Star), R(1),
@@ -215,7 +215,7 @@ frame size: 2
 parameter count: 1
 bytecode array length: 16
 bytecodes: [
-  /*   53 S> */ B(Wide), B(LdaSmi), I16(265),
+  /*   53 S> */ B(Wide), B(LdaSmi), I16(266),
                 B(Star), R(0),
                 B(LdaConstant), U8(0),
                 B(Star), R(1),
@@ -241,7 +241,7 @@ frame size: 2
 parameter count: 1
 bytecode array length: 16
 bytecodes: [
-  /*   60 S> */ B(Wide), B(LdaSmi), I16(266),
+  /*   60 S> */ B(Wide), B(LdaSmi), I16(267),
                 B(Star), R(0),
                 B(LdaConstant), U8(0),
                 B(Star), R(1),
@@ -267,7 +267,7 @@ frame size: 3
 parameter count: 1
 bytecode array length: 16
 bytecodes: [
-  /*   46 S> */ B(Wide), B(LdaSmi), I16(265),
+  /*   46 S> */ B(Wide), B(LdaSmi), I16(266),
                 B(Star), R(1),
                 B(LdaConstant), U8(0),
                 B(Star), R(2),

MylesBorins · 2020-10-21T20:01:35Z

Another V8 CI https://ci.nodejs.org/job/node-test-commit-v8-linux/3501/

Original commit message: parser: better error message for await+tla Bug: v8:9344, v8:6513 Change-Id: I1854e483515e7da99192367b6764a0ec7c8b41d9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2411687 Reviewed-by: Marja Hölttä <marja@chromium.org> Commit-Queue: Gus Caplan <snek@chromium.org> Cr-Commit-Position: refs/heads/master@{#70099} Refs: v8/v8@4263f8a PR-URL: #35650 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>

nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Oct 14, 2020

MylesBorins mentioned this pull request Oct 14, 2020

module: Improves Top-Level await error in cjs #35196

Closed

4 tasks

MylesBorins reviewed Oct 14, 2020

View reviewed changes

Trott force-pushed the bdougie-v8-backport branch 2 times, most recently from 678b58e to eb1aac1 Compare October 21, 2020 13:11

Trott approved these changes Oct 21, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

joyeecheung reviewed Oct 21, 2020

View reviewed changes

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden Outdated Show resolved Hide resolved

MylesBorins reviewed Oct 21, 2020

View reviewed changes

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden Outdated Show resolved Hide resolved

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden Outdated Show resolved Hide resolved

MylesBorins reviewed Oct 21, 2020

View reviewed changes

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden Outdated Show resolved Hide resolved

deps/v8/test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden Outdated Show resolved Hide resolved

MylesBorins force-pushed the bdougie-v8-backport branch from c7cabcf to ff9c811 Compare October 21, 2020 16:14

fixup! deps: V8: backport 4263f8a5e8e0

dd5deaf

MylesBorins added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 21, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 21, 2020

targos mentioned this pull request Nov 3, 2020

v15.1.0 release proposal #35948

Merged

targos added the dont-land-on-v14.x label Apr 30, 2021

This was referenced Aug 16, 2022

[Snyk] Security upgrade gatsby-source-filesystem from 2.1.43 to 4.21.0 Rednickle/node-1#52

Open

[Snyk] Security upgrade gatsby-source-filesystem from 2.1.43 to 4.21.0 aliceUnhinged613/node#65

Open

Dmarch28 mentioned this pull request Aug 16, 2022

[Snyk] Security upgrade gatsby-source-filesystem from 2.3.23 to 4.21.0 Dmarch28/node#63

Open

wedataintelligence mentioned this pull request Aug 16, 2022

[Snyk] Security upgrade gatsby-source-filesystem from 2.1.43 to 4.21.0 wedataintelligence/node#39

Open

kissofire mentioned this pull request Aug 16, 2022

[Snyk] Security upgrade gatsby-source-filesystem from 2.1.21 to 4.21.0 kissofire/node#74

Open

molllyn1 mentioned this pull request Aug 17, 2022

[Snyk] Security upgrade gatsby-source-filesystem from 2.1.43 to 4.21.0 molllyn1/node#54

Open

snyk-bot mentioned this pull request Aug 17, 2022

[Snyk] Security upgrade gatsby-source-filesystem from 2.1.43 to 4.21.0 137717unity/node#63

Open

baby636 mentioned this pull request Aug 17, 2022

[Snyk] Security upgrade gatsby-source-filesystem from 2.3.23 to 4.21.0 baby636/node#63

Open

ron-a20 mentioned this pull request Aug 17, 2022

[Snyk] Security upgrade gatsby-source-filesystem from 2.1.43 to 4.21.0 ron-a20/node#72

Open

snyk-bot mentioned this pull request Sep 25, 2022

[Snyk] Fix for 22 vulnerabilities 137717unity/node#70

Open

wedataintelligence mentioned this pull request Sep 26, 2022

[Snyk] Fix for 22 vulnerabilities wedataintelligence/node#44

Open

137717unity mentioned this pull request Sep 26, 2022

[Snyk] Fix for 22 vulnerabilities 137717unity/node#73

Open

snyk-bot mentioned this pull request Sep 27, 2022

[Snyk] Fix for 21 vulnerabilities Rednickle/node-1#56

Open

wedataintelligence mentioned this pull request Sep 29, 2022

[Snyk] Fix for 24 vulnerabilities wedataintelligence/node#50

Open

This was referenced Sep 29, 2022

[Snyk] Fix for 24 vulnerabilities 137717unity/node#75

Open

[Snyk] Fix for 23 vulnerabilities Rednickle/node-1#60

Open

This was referenced Oct 5, 2022

[Snyk] Fix for 24 vulnerabilities wedataintelligence/node#51

Open

[Snyk] Fix for 24 vulnerabilities wedataintelligence/node#52

Open

snyk-bot mentioned this pull request Oct 6, 2022

[Snyk] Fix for 24 vulnerabilities wedataintelligence/node#53

Open

137717unity mentioned this pull request Oct 7, 2022

[Snyk] Fix for 24 vulnerabilities 137717unity/node#76

Open

wedataintelligence mentioned this pull request Oct 7, 2022

[Snyk] Fix for 24 vulnerabilities wedataintelligence/node#54

Open

This was referenced Oct 8, 2022

[Snyk] Fix for 24 vulnerabilities wedataintelligence/node#55

Open

[Snyk] Fix for 24 vulnerabilities 137717unity/node#77

Open

137717unity mentioned this pull request Oct 9, 2022

[Snyk] Fix for 24 vulnerabilities 137717unity/node#78

Open

snyk-bot mentioned this pull request Oct 9, 2022

[Snyk] Fix for 23 vulnerabilities Rednickle/node-1#63

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: V8: backport 4263f8a5e8e0 #35650

deps: V8: backport 4263f8a5e8e0 #35650

bdougie commented Oct 14, 2020

bdougie commented Oct 14, 2020 •

edited

Loading

nodejs-github-bot commented Oct 14, 2020 •

edited by MylesBorins

Loading

MylesBorins left a comment

nodejs-github-bot commented Oct 14, 2020 •

edited by MylesBorins

Loading

MylesBorins commented Oct 15, 2020

bdougie commented Oct 15, 2020 •

edited

Loading

MylesBorins commented Oct 16, 2020

Trott commented Oct 21, 2020 •

edited

Loading

Trott commented Oct 21, 2020

Trott left a comment

This comment has been minimized.

MylesBorins commented Oct 21, 2020

joyeecheung commented Oct 21, 2020

joyeecheung commented Oct 21, 2020 •

edited

Loading

MylesBorins commented Oct 21, 2020 •

edited

Loading

joyeecheung commented Oct 21, 2020 •

edited

Loading

MylesBorins commented Oct 21, 2020 •

edited

Loading

deps: V8: backport 4263f8a5e8e0 #35650

deps: V8: backport 4263f8a5e8e0 #35650

Conversation

bdougie commented Oct 14, 2020

Checklist

bdougie commented Oct 14, 2020 • edited Loading

nodejs-github-bot commented Oct 14, 2020 • edited by MylesBorins Loading

MylesBorins left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Oct 14, 2020 • edited by MylesBorins Loading

MylesBorins commented Oct 15, 2020

bdougie commented Oct 15, 2020 • edited Loading

MylesBorins commented Oct 16, 2020

Trott commented Oct 21, 2020 • edited Loading

Trott commented Oct 21, 2020

Trott left a comment

Choose a reason for hiding this comment

This comment has been minimized.

MylesBorins commented Oct 21, 2020

joyeecheung commented Oct 21, 2020

joyeecheung commented Oct 21, 2020 • edited Loading

MylesBorins commented Oct 21, 2020 • edited Loading

joyeecheung commented Oct 21, 2020 • edited Loading

MylesBorins commented Oct 21, 2020 • edited Loading

bdougie commented Oct 14, 2020 •

edited

Loading

nodejs-github-bot commented Oct 14, 2020 •

edited by MylesBorins

Loading

nodejs-github-bot commented Oct 14, 2020 •

edited by MylesBorins

Loading

bdougie commented Oct 15, 2020 •

edited

Loading

Trott commented Oct 21, 2020 •

edited

Loading

joyeecheung commented Oct 21, 2020 •

edited

Loading

MylesBorins commented Oct 21, 2020 •

edited

Loading

joyeecheung commented Oct 21, 2020 •

edited

Loading

MylesBorins commented Oct 21, 2020 •

edited

Loading