Skip to content

src: use new V8 API in vm module#16293

Closed
fhinkel wants to merge 1 commit intonodejs:masterfrom
fhinkel:Oct/defineProp
Closed

src: use new V8 API in vm module#16293
fhinkel wants to merge 1 commit intonodejs:masterfrom
fhinkel:Oct/defineProp

Conversation

@fhinkel
Copy link
Copy Markdown
Member

@fhinkel fhinkel commented Oct 18, 2017

This PR removes the CopyProperties() hack in the vm module, i.e., Contextify.
Instead, it uses different V8 API methods, that allow interception of
DefineProperty() and do not flatten accessor descriptors to
property descriptors.

Move many known issues to test cases. Factor out the last test in
test-vm-context.js for
#10223
into its own file, test-vm-strict-assign.js.

Part of this CL is taken from a stalled PR by
https://github.com/AnnaMag
#13265

Refs: #6283
Refs: #15114
Refs: #13265

Fixes: #2734
Fixes: #10223
Fixes: #11803
Fixes: #11902

This PR requires a backport of
37a3a15 otherwise some tests will fail. Cherry pick PR.

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
Affected core subsystem(s)

src

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
Affected core subsystem(s)

src

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. vm Issues and PRs related to the vm subsystem.

Projects

None yet

10 participants