Implement Runtime Private Environments #2929

raskad · 2023-05-13T02:57:39Z

This Pull Request changes the following:

Implement runtime private environments and private names conforming with the spec.
Remove parse-time distinction and mutation of private names.
Adjust early error handling for private names to conform with the spec.

github-actions · 2023-05-13T03:08:56Z

Test262 conformance changes

Test result	main count	PR count	difference
Total	93,982	93,982	0
Passed	73,694	73,783	+89
Ignored	17,489	17,489	0
Failed	2,799	2,710	-89
Panics	0	0	0
Conformance	78.41%	78.51%	+0.09%

Fixed tests (89):

test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-factory.js [strict mode] (previously Failed)
test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-factory.js (previously Failed)
test/language/expressions/class/private-static-getter-multiple-evaluations-of-class-realm.js (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-realm-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-realm-function-ctor.js (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-realm.js (previously Failed)
test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-eval-indirect.js (previously Failed)
test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-realm-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-realm-function-ctor.js (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-eval-indirect.js (previously Failed)
test/language/expressions/class/private-static-field-multiple-evaluations-of-class-realm.js (previously Failed)
test/language/expressions/class/private-static-method-brand-check-multiple-evaluations-of-class-realm.js (previously Failed)
test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-eval.js (previously Failed)
test/language/expressions/class/private-static-field-multiple-evaluations-of-class-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-static-field-multiple-evaluations-of-class-function-ctor.js (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-function-ctor.js (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-eval-indirect.js (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-factory.js [strict mode] (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-factory.js (previously Failed)
test/language/expressions/class/private-static-field-multiple-evaluations-of-class-eval-indirect.js (previously Failed)
test/language/expressions/class/private-static-setter-multiple-evaluations-of-class-factory.js [strict mode] (previously Failed)
test/language/expressions/class/private-static-setter-multiple-evaluations-of-class-factory.js (previously Failed)
test/language/expressions/class/private-static-setter-multiple-evaluations-of-class-eval-indirect.js (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-eval.js (previously Failed)
test/language/expressions/class/private-static-setter-multiple-evaluations-of-class-realm.js (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-factory.js [strict mode] (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-factory.js (previously Failed)
test/language/expressions/class/private-static-setter-multiple-evaluations-of-class-direct-eval.js (previously Failed)
test/language/expressions/class/private-static-field-multiple-evaluations-of-class-factory.js [strict mode] (previously Failed)
test/language/expressions/class/private-static-field-multiple-evaluations-of-class-factory.js (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-function-ctor.js (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-eval.js (previously Failed)
test/language/expressions/class/private-static-method-brand-check-multiple-evaluations-of-class-factory.js [strict mode] (previously Failed)
test/language/expressions/class/private-static-method-brand-check-multiple-evaluations-of-class-factory.js (previously Failed)
test/language/expressions/class/private-static-field-multiple-evaluations-of-class-direct-eval.js (previously Failed)
test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-realm.js (previously Failed)
test/language/expressions/class/private-static-getter-multiple-evaluations-of-class-eval-indirect.js (previously Failed)
test/language/expressions/class/private-static-getter-multiple-evaluations-of-class-direct-eval.js (previously Failed)
test/language/expressions/class/private-method-brand-check-multiple-evaluations-of-class-realm.js (previously Failed)
test/language/expressions/class/private-static-method-brand-check-multiple-evaluations-of-class-direct-eval.js (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-realm-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-setter-brand-check-multiple-evaluations-of-class-realm-function-ctor.js (previously Failed)
test/language/expressions/class/private-static-getter-multiple-evaluations-of-class-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-static-getter-multiple-evaluations-of-class-function-ctor.js (previously Failed)
test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-getter-brand-check-multiple-evaluations-of-class-function-ctor.js (previously Failed)
test/language/expressions/class/private-static-setter-multiple-evaluations-of-class-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-static-setter-multiple-evaluations-of-class-function-ctor.js (previously Failed)
test/language/expressions/class/private-static-method-brand-check-multiple-evaluations-of-class-eval-indirect.js (previously Failed)
test/language/expressions/class/private-static-getter-multiple-evaluations-of-class-factory.js [strict mode] (previously Failed)
test/language/expressions/class/private-static-getter-multiple-evaluations-of-class-factory.js (previously Failed)
test/language/expressions/class/private-static-method-brand-check-multiple-evaluations-of-class-function-ctor.js [strict mode] (previously Failed)
test/language/expressions/class/private-static-method-brand-check-multiple-evaluations-of-class-function-ctor.js (previously Failed)
test/language/statements/class/elements/private-method-brand-check-multiple-evaluations-of-class.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-method-brand-check-multiple-evaluations-of-class.js (previously Failed)
test/language/statements/class/elements/private-setter-visible-to-direct-eval.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-setter-visible-to-direct-eval.js (previously Failed)
test/language/statements/class/elements/private-getter-visible-to-direct-eval-on-initializer.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-getter-visible-to-direct-eval-on-initializer.js (previously Failed)
test/language/statements/class/elements/privatefieldset-typeerror-4.js [strict mode] (previously Failed)
test/language/statements/class/elements/privatefieldset-typeerror-4.js (previously Failed)
test/language/statements/class/elements/private-method-visible-to-direct-eval-on-initializer.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-method-visible-to-direct-eval-on-initializer.js (previously Failed)
test/language/statements/class/elements/private-setter-brand-check-multiple-evaluations-of-class.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-setter-brand-check-multiple-evaluations-of-class.js (previously Failed)
test/language/statements/class/elements/private-setter-visible-to-direct-eval-on-initializer.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-setter-visible-to-direct-eval-on-initializer.js (previously Failed)
test/language/statements/class/elements/private-static-field-visible-to-direct-eval.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-static-field-visible-to-direct-eval.js (previously Failed)
test/language/statements/class/elements/privatefieldget-typeerror-4.js [strict mode] (previously Failed)
test/language/statements/class/elements/privatefieldget-typeerror-4.js (previously Failed)
test/language/statements/class/elements/private-static-getter-visible-to-direct-eval.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-static-getter-visible-to-direct-eval.js (previously Failed)
test/language/statements/class/elements/private-field-visible-to-direct-eval-on-initializer.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-field-visible-to-direct-eval-on-initializer.js (previously Failed)
test/language/statements/class/elements/private-method-visible-to-direct-eval.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-method-visible-to-direct-eval.js (previously Failed)
test/language/statements/class/elements/private-getter-visible-to-direct-eval.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-getter-visible-to-direct-eval.js (previously Failed)
test/language/statements/class/elements/private-static-setter-visible-to-direct-eval.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-static-setter-visible-to-direct-eval.js (previously Failed)
test/language/statements/class/elements/private-static-method-visible-to-direct-eval.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-static-method-visible-to-direct-eval.js (previously Failed)
test/language/statements/class/elements/private-field-visible-to-direct-eval.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-field-visible-to-direct-eval.js (previously Failed)
test/language/statements/class/elements/private-getter-brand-check-multiple-evaluations-of-class.js [strict mode] (previously Failed)
test/language/statements/class/elements/private-getter-brand-check-multiple-evaluations-of-class.js (previously Failed)

codecov · 2023-05-13T03:18:18Z

Codecov Report

Merging #2929 (7c466f7) into main (ff42086) will decrease coverage by 0.07%.
The diff coverage is 44.82%.

@@            Coverage Diff             @@
##             main    #2929      +/-   ##
==========================================
- Coverage   50.45%   50.39%   -0.07%     
==========================================
  Files         442      443       +1     
  Lines       45267    45336      +69     
==========================================
+ Hits        22840    22846       +6     
- Misses      22427    22490      +63

Impacted Files	Coverage Δ
boa_ast/src/function/class.rs	`70.00% <ø> (-0.12%)`	⬇️
boa_engine/src/object/jsobject.rs	`58.64% <0.00%> (-0.67%)`	⬇️
boa_engine/src/object/mod.rs	`32.51% <0.00%> (-0.05%)`	⬇️
boa_engine/src/object/operations.rs	`52.84% <ø> (ø)`
boa_engine/src/vm/code_block.rs	`55.80% <0.00%> (-0.29%)`	⬇️
boa_engine/src/vm/flowgraph/mod.rs	`0.00% <0.00%> (ø)`
boa_engine/src/vm/opcode/binary_ops/mod.rs	`66.03% <0.00%> (-5.40%)`	⬇️
boa_engine/src/vm/opcode/get/private.rs	`0.00% <0.00%> (ø)`
boa_engine/src/vm/opcode/mod.rs	`45.45% <ø> (ø)`
boa_engine/src/vm/opcode/push/class/field.rs	`0.00% <0.00%> (ø)`
... and 17 more

... and 1 file with indirect coverage changes

HalidOdat

Overall it looks good! Just had a small concern about the id field. The id (pointer address) is valid only for how long the Object is (because it can be reassigned to another object).

Can it be used after the object is deallocated?

HalidOdat · 2023-05-13T23:33:55Z

boa_engine/src/vm/code_block.rs

+                *pc += size_of::<u32>() * (count as usize + 1);
+                String::new()


It would be nice to print the operands

raskad · 2023-05-14T02:19:25Z

Overall it looks good! Just had a small concern about the id field. The id (pointer address) is valid only for how long the Object is (because it can be reassigned to another object).

Can it be used after the object is deallocated?

I thought a bit about this. I think this should work. The private environments are only stored on function objects that are methods of classes. Those function objects always have a reference back to the class object with the class_object field (I think in theory the class object should be in the environment but that leads to the same outcome). So as long as the function objects with the private environments are alive, the class objects with the id pointer must also be alive.

HalidOdat

Looks perfect to me!

jedel1043

Great work!

raskad added bug Something isn't working parser Issues surrounding the parser execution Issues or PRs related to code execution ast Issue surrounding the abstract syntax tree labels May 13, 2023

raskad added this to the v0.17.0 milestone May 13, 2023

raskad requested a review from a team May 13, 2023 03:10

HalidOdat reviewed May 13, 2023

View reviewed changes

HalidOdat approved these changes May 14, 2023

View reviewed changes

jedel1043 approved these changes May 16, 2023

View reviewed changes

Implement Private Runtime Environments

7c466f7

raskad force-pushed the private-environments branch from 2d3b13d to 7c466f7 Compare May 18, 2023 20:40

raskad added this pull request to the merge queue May 18, 2023

Merged via the queue into main with commit 5e9193a May 18, 2023

raskad deleted the private-environments branch May 18, 2023 21:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Runtime Private Environments #2929

Implement Runtime Private Environments #2929

raskad commented May 13, 2023

github-actions bot commented May 13, 2023 •

edited

Loading

codecov bot commented May 13, 2023 •

edited

Loading

HalidOdat left a comment

HalidOdat May 13, 2023

raskad commented May 14, 2023

HalidOdat left a comment

jedel1043 left a comment

		pc += size_of::<u32>() (count as usize + 1);
		String::new()

Implement Runtime Private Environments #2929

Implement Runtime Private Environments #2929

Conversation

raskad commented May 13, 2023

github-actions bot commented May 13, 2023 • edited Loading

Test262 conformance changes

codecov bot commented May 13, 2023 • edited Loading

Codecov Report

HalidOdat left a comment

Choose a reason for hiding this comment

HalidOdat May 13, 2023

Choose a reason for hiding this comment

raskad commented May 14, 2023

HalidOdat left a comment

Choose a reason for hiding this comment

jedel1043 left a comment

Choose a reason for hiding this comment

github-actions bot commented May 13, 2023 •

edited

Loading

codecov bot commented May 13, 2023 •

edited

Loading