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

Refactor EnvironmentRecordTrait functions #1569

Merged
merged 1 commit into from
Sep 15, 2021
Merged

Refactor EnvironmentRecordTrait functions #1569

merged 1 commit into from
Sep 15, 2021

Conversation

raskad
Copy link
Member

@raskad raskad commented Sep 9, 2021

It changes the following:

  • Refactor some EnvironmentRecordTrait function implementations for spec compliance.
  • Change some EnvironmentRecordTrait functions to take &str instead of String to avoid allocations.
  • Change some EnvironmentRecordTrait functions to use accept Context and return JsResult for some spec compliant error handling.
  • Add documentation for EnvironmentRecordTrait function implementations.
  • Change ObjectEnvironmentRecord.bindings from Jsvalue to JsObject
  • Remove now unused JsValue.is_extensible and JsValue.has_field functions.
  • Replace the static strict = true in some Context.set_mutable_binding calls with the current strict value of Context.

@raskad raskad added the execution Issues or PRs related to code execution label Sep 9, 2021
@raskad
Copy link
Member Author

raskad commented Sep 9, 2021 •
edited
Loading

Test262 conformance changes:

Test result master count PR count difference
Total 80,804 80,804 0
Passed 33,132 33,162 +30
Ignored 15,896 15,896 0
Failed 31,776 31,746 -30
Panics 0 0 0
Conformance 41.00% 41.04% +0.04%
Fixed tests (30): 
test/language/expressions/prefix-decrement/operator-prefix-decrement-x-calls-putvalue-lhs-newvalue--1.js (previously Failed)
test/language/expressions/postfix-increment/operator-x-postfix-increment-calls-putvalue-lhs-newvalue--1.js (previously Failed)
test/language/expressions/postfix-decrement/operator-x-postfix-decrement-calls-putvalue-lhs-newvalue--1.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--1.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--17.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--21.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--5.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--15.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--9.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--19.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--7.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--3.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--11.js (previously Failed)
test/language/expressions/compound-assignment/compound-assignment-operator-calls-putvalue-lref--v--13.js (previously Failed)
test/language/expressions/prefix-increment/operator-prefix-increment-x-calls-putvalue-lhs-newvalue--1.js (previously Failed)
test/language/expressions/typeof/get-value.js [strict mode] (previously Failed)
test/language/expressions/typeof/get-value.js (previously Failed)
test/language/statements/const/syntax/const-invalid-assignment-next-expression-for.js [strict mode] (previously Failed)
test/language/statements/const/syntax/const-invalid-assignment-next-expression-for.js (previously Failed)
test/language/statements/const/syntax/const-invalid-assignment-statement-body-for-in.js [strict mode] (previously Failed)
test/language/statements/const/syntax/const-invalid-assignment-statement-body-for-in.js (previously Failed)
test/language/statements/const/syntax/const-invalid-assignment-statement-body-for-of.js [strict mode] (previously Failed)
test/language/statements/const/syntax/const-invalid-assignment-statement-body-for-of.js (previously Failed)
test/built-ins/global/10.2.1.1.3-4-18-s.js [strict mode] (previously Failed)
test/built-ins/global/10.2.1.1.3-4-16-s.js [strict mode] (previously Failed)
test/built-ins/JSON/parse/text-negative-zero.js [strict mode] (previously Failed)
test/built-ins/JSON/parse/text-negative-zero.js (previously Failed)
test/built-ins/Infinity/S15.1.1.2_A2_T2.js (previously Failed)
test/built-ins/undefined/15.1.1.3-1.js (previously Failed)
test/built-ins/NaN/S15.1.1.1_A2_T2.js (previously Failed)

jedel1043
jedel1043 requested changes Sep 10, 2021
View reviewed changes
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Just some small details about property descriptors. Good work!

boa/src/environment/global_environment_record.rs Outdated Show resolved Hide resolved
boa/src/environment/global_environment_record.rs Outdated Show resolved Hide resolved
boa/src/environment/global_environment_record.rs Outdated Show resolved Hide resolved
jedel1043
jedel1043 approved these changes Sep 10, 2021
View reviewed changes
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

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

Looks good!

@raskad
Copy link
Member Author

raskad commented Sep 15, 2021

Rebased

@raskad raskad changed the title Refactor some EnvironmentRecordTrait functions Refactor EnvironmentRecordTrait functions Sep 15, 2021
@raskad raskad merged commit c5a4be0 into boa-dev:master Sep 15, 2021
@raskad raskad added this to the v0.13.0 milestone Sep 15, 2021
@HalidOdat HalidOdat linked an issue Sep 15, 2021 that may be closed by this pull request
panicked at 'assertion failed: value.is_object() || value.is_function()' #1404
Closed
@HalidOdat
Copy link
Member

This also fixes #1404

@HalidOdat HalidOdat mentioned this pull request Sep 15, 2021
Closed
@raskad raskad deleted the env-refactor branch September 15, 2021 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RageKnify RageKnify RageKnify approved these changes

@jedel1043 jedel1043 jedel1043 approved these changes

@Razican Razican Awaiting requested review from Razican

@HalidOdat HalidOdat Awaiting requested review from HalidOdat

Assignees
No one assigned
Labels
execution Issues or PRs related to code execution
Projects
None yet
Milestone
v0.13.0
Development

Successfully merging this pull request may close these issues.

panicked at 'assertion failed: value.is_object() || value.is_function()'
4 participants
@raskad @HalidOdat @RageKnify @jedel1043