Skip to content

Fix the private field crash for the Array object.#5139

Merged
LaszloLango merged 1 commit into
jerryscript-project:masterfrom
applepiep1e:fix-private-field
Nov 25, 2024
Merged

Fix the private field crash for the Array object.#5139
LaszloLango merged 1 commit into
jerryscript-project:masterfrom
applepiep1e:fix-private-field

Conversation

@applepiep1e

@applepiep1e applepiep1e commented Apr 20, 2024

Copy link
Copy Markdown
Contributor

Fix the private field crash in the Array object mentioned in
#5138

@zherczeg

zherczeg commented May 3, 2024

Copy link
Copy Markdown
Member

Good patch. Please add the testcase.

@applepiep1e

applepiep1e commented May 3, 2024

Copy link
Copy Markdown
Contributor Author

Good patch. Please add the testcase.

Added.

zherczeg
zherczeg approved these changes May 4, 2024
View reviewed changes

@zherczeg zherczeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@zherczeg

zherczeg commented May 4, 2024

Copy link
Copy Markdown
Member

Please squash the commits and add DCO to the commit message.

@applepiep1e
applepiep1e force-pushed the fix-private-field branch 2 times, most recently from c7dcf9a to 2c0f9d2 Compare May 7, 2024 02:45
akosthekiss
akosthekiss previously requested changes May 7, 2024
View reviewed changes

@akosthekiss akosthekiss left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The code seems OK, but the style of the test case addition is completely out of sync with the existing parts of the test case. Should be aligned.

Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
Comment thread tests/jerry/private_fields.js Outdated
@applepiep1e
applepiep1e force-pushed the fix-private-field branch from 04beb27 to 411a949 Compare May 8, 2024 04:39
@LaszloLango

LaszloLango commented Nov 12, 2024

Copy link
Copy Markdown
Contributor

@akosthekiss Please review the latest changes.

@ossy-szeged ossy-szeged added this to the Release 3.0.0 milestone Nov 12, 2024
@LaszloLango

LaszloLango commented Nov 22, 2024

Copy link
Copy Markdown
Contributor

We should check whether this fixes #5097 and #5100

@robertsipka

robertsipka commented Nov 25, 2024

Copy link
Copy Markdown
Contributor

I've updated the test cases and rebased to the master.
This PR fixes #5097 and #5100 and #5138 as well.

@LaszloLango

LaszloLango commented Nov 25, 2024

Copy link
Copy Markdown
Contributor

@robertsipka could you please add new regression tests with the issue numbers for all three?

@robertsipka

robertsipka commented Nov 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

@robertsipka could you please add new regression tests with the issue numbers for all three?

I've followed the author's original intention and amended the other two test cases to the test case which was originally included in the PR, but It's actually easier to follow if they are added as regression tests, I modified it.

@applepiep1e @robertsipka
Fix the private field crash for the Array object.
cab30e5 
Fixes the following issues: 5097, 5100, 5138

Additional regression test cases added by: Robert Sipka <robert.sipka@h-lab.eu>

JerryScript-DCO-1.0-Signed-off-by: Baihe Jiang <baihe.jiang@outlook.com>
JerryScript-DCO-1.0-Signed-off-by: Lily Jiang bhjiang.whu@hotmail.com
@LaszloLango
LaszloLango dismissed akosthekiss’s stale review November 25, 2024 12:03

Requested change has been done.

@LaszloLango
LaszloLango merged commit 95cc5e9 into jerryscript-project:master Nov 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zherczeg zherczeg zherczeg approved these changes
@LaszloLango LaszloLango LaszloLango approved these changes
@akosthekiss akosthekiss Awaiting requested review from akosthekiss

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Release 3.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@applepiep1e @zherczeg @LaszloLango @robertsipka @akosthekiss @ossy-szeged