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

[Merged by Bors] - %NativeError%.[[prototype]] should be Error constructor #1883

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - %NativeError%.[[prototype]] should be Error constructor #1883

wants to merge 1 commit into from

Conversation

HalidOdat
Copy link
Member

@HalidOdat HalidOdat commented Mar 1, 2022 •
edited by RageKnify
Loading

Before the %NativeError% objects (like TypeError, ReferenceError, etc) [[prototype]] field was set to Function.prototype but this is wrong it should be the Error constructor object itself.

This makes the %NativeError%s 100% spec compliant :)
(except AggregateError because its not implemented)

@HalidOdat HalidOdat added bug Something isn't working builtins PRs and Issues related to builtins/intrinsics labels Mar 1, 2022
@HalidOdat HalidOdat added this to the v0.14.0 milestone Mar 1, 2022
@github-actions
Copy link

github-actions bot commented Mar 1, 2022

Test262 conformance changes

VM implementation

Test result main count PR count difference
Total 88,342 88,342 0
Passed 43,041 43,065 +24
Ignored 21,413 21,413 0
Failed 23,888 23,864 -24
Panics 0 0 0
Conformance 48.72% 48.75% +0.03%
Fixed tests (24): 
test/built-ins/NativeErrors/RangeError/proto.js [strict mode] (previously Failed)
test/built-ins/NativeErrors/RangeError/proto.js (previously Failed)
test/built-ins/NativeErrors/ReferenceError/proto.js [strict mode] (previously Failed)
test/built-ins/NativeErrors/ReferenceError/proto.js (previously Failed)
test/built-ins/NativeErrors/SyntaxError/proto.js [strict mode] (previously Failed)
test/built-ins/NativeErrors/SyntaxError/proto.js (previously Failed)
test/built-ins/NativeErrors/URIError/proto.js [strict mode] (previously Failed)
test/built-ins/NativeErrors/URIError/proto.js (previously Failed)
test/built-ins/NativeErrors/TypeError/proto.js [strict mode] (previously Failed)
test/built-ins/NativeErrors/TypeError/proto.js (previously Failed)
test/built-ins/NativeErrors/EvalError/proto.js [strict mode] (previously Failed)
test/built-ins/NativeErrors/EvalError/proto.js (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-16.js [strict mode] (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-16.js (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-12.js [strict mode] (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-12.js (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-14.js [strict mode] (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-14.js (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-15.js [strict mode] (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-15.js (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-17.js [strict mode] (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-17.js (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-13.js [strict mode] (previously Failed)
test/built-ins/Object/getPrototypeOf/15.2.3.2-2-13.js (previously Failed)

@codecov
Copy link

codecov bot commented Mar 1, 2022 •
edited
Loading

Codecov Report

Merging #1883 (de137b2) into main (b2d3720) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1883      +/-   ##
==========================================
+ Coverage   46.73%   46.77%   +0.04%     
==========================================
  Files         204      204              
  Lines       16732    16746      +14     
==========================================
+ Hits         7819     7833      +14     
  Misses       8913     8913
Impacted Files Coverage Δ
boa_engine/src/builtins/error/eval.rs 100.00% <100.00%> (ø)
boa_engine/src/builtins/error/range.rs 100.00% <100.00%> (ø)
boa_engine/src/builtins/error/reference.rs 100.00% <100.00%> (ø)
boa_engine/src/builtins/error/syntax.rs 100.00% <100.00%> (ø)
boa_engine/src/builtins/error/type.rs 100.00% <100.00%> (ø)
boa_engine/src/builtins/error/uri.rs 100.00% <100.00%> (ø)
boa_engine/src/vm/mod.rs 70.79% <0.00%> (+0.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2d3720...de137b2. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Mar 2, 2022

Benchmark for 435c08e

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 437.5±5.68ns 445.2±8.85ns +1.76%
Arithmetic operations (Execution) 1696.4±26.26ns 1748.3±26.29ns +3.06%
Arithmetic operations (Parser) 4.5±0.06µs 4.5±0.07µs 0.00%
Array access (Compiler) 1077.6±12.47ns 1070.6±21.46ns -0.65%
Array access (Execution) 8.4±0.14µs 8.4±0.13µs 0.00%
Array access (Parser) 9.8±0.13µs 9.8±0.15µs 0.00%
Array creation (Compiler) 1561.1±20.19ns 1546.3±21.70ns -0.95%
Array creation (Execution) 2.7±0.04ms 2.7±0.04ms 0.00%
Array creation (Parser) 10.9±0.15µs 11.1±0.17µs +1.83%
Array pop (Compiler) 3.3±0.04µs 3.4±0.05µs +3.03%
Array pop (Execution) 1177.2±18.34µs 1183.1±15.93µs +0.50%
Array pop (Parser) 111.8±1.63µs 112.6±1.80µs +0.72%
Boolean Object Access (Compiler) 943.3±14.22ns 945.5±16.49ns +0.23%
Boolean Object Access (Execution) 4.9±0.08µs 5.3±0.08µs +8.16%
Boolean Object Access (Parser) 11.8±0.18µs 11.9±0.16µs +0.85%
Clean js (Compiler) 2.9±0.05µs 2.9±0.05µs 0.00%
Clean js (Execution) 939.6±13.32µs 951.4±14.29µs +1.26%
Clean js (Parser) 24.0±0.37µs 23.9±0.34µs -0.42%
Create Realm 230.1±4.66ns 235.5±3.65ns +2.35%
Dynamic Object Property Access (Compiler) 1340.6±21.20ns 1378.8±21.46ns +2.85%
Dynamic Object Property Access (Execution) 5.9±0.10µs 5.8±0.09µs -1.69%
Dynamic Object Property Access (Parser) 8.7±0.13µs 8.8±0.13µs +1.15%
Fibonacci (Compiler) 1911.0±28.00ns 1931.0±33.63ns +1.05%
Fibonacci (Execution) 1509.1±22.57µs 1518.5±24.33µs +0.62%
Fibonacci (Parser) 13.4±0.20µs 13.4±0.19µs 0.00%
For loop (Compiler) 1672.9±29.98ns 1662.6±24.89ns -0.62%
For loop (Execution) 36.0±0.59µs 36.2±0.52µs +0.56%
For loop (Parser) 11.4±0.18µs 11.5±0.19µs +0.88%
Mini js (Compiler) 2.8±0.04µs 2.8±0.04µs 0.00%
Mini js (Execution) 880.1±11.96µs 881.3±11.61µs +0.14%
Mini js (Parser) 21.1±0.39µs 20.9±0.29µs -0.95%
Number Object Access (Compiler) 879.0±11.89ns 887.5±15.79ns +0.97%
Number Object Access (Execution) 3.9±0.06µs 4.2±0.05µs +7.69%
Number Object Access (Parser) 9.2±0.14µs 9.2±0.14µs 0.00%
Object Creation (Compiler) 1169.5±18.13ns 1170.4±16.48ns +0.08%
Object Creation (Execution) 5.2±0.09µs 5.3±0.09µs +1.92%
Object Creation (Parser) 7.7±0.14µs 7.6±0.14µs -1.30%
RegExp (Compiler) 1382.6±24.55ns 1401.0±27.46ns +1.33%
RegExp (Execution) 10.9±0.18µs 10.6±0.16µs -2.75%
RegExp (Parser) 8.3±0.13µs 8.4±0.13µs +1.20%
RegExp Creation (Compiler) 1225.0±2.94ns 1214.2±17.29ns -0.88%
RegExp Creation (Execution) 8.0±0.14µs 7.7±0.11µs -3.75%
RegExp Creation (Parser) 6.9±0.11µs 7.0±0.10µs +1.45%
RegExp Literal (Compiler) 1385.5±18.44ns 1395.6±18.96ns +0.73%
RegExp Literal (Execution) 11.0±0.14µs 10.6±0.19µs -3.64%
RegExp Literal (Parser) 6.8±0.09µs 6.9±0.08µs +1.47%
RegExp Literal Creation (Compiler) 1235.8±3.06ns 1206.5±20.12ns -2.37%
RegExp Literal Creation (Execution) 8.2±0.11µs 7.8±0.08µs -4.88%
RegExp Literal Creation (Parser) 5.4±0.06µs 5.3±0.09µs -1.85%
Static Object Property Access (Compiler) 1216.9±18.24ns 1202.2±18.67ns -1.21%
Static Object Property Access (Execution) 5.5±0.09µs 5.5±0.09µs 0.00%
Static Object Property Access (Parser) 8.2±0.14µs 8.2±0.13µs 0.00%
String Object Access (Compiler) 1256.4±19.25ns 1283.5±72.37ns +2.16%
String Object Access (Execution) 6.6±0.09µs 7.0±0.11µs +6.06%
String Object Access (Parser) 12.3±6.06µs 11.7±0.18µs -4.88%
String comparison (Compiler) 1747.4±23.27ns 1824.1±27.66ns +4.39%
String comparison (Execution) 5.0±0.08µs 5.0±0.08µs 0.00%
String comparison (Parser) 9.1±0.13µs 9.1±0.16µs 0.00%
String concatenation (Compiler) 1364.3±23.30ns 1404.4±21.79ns +2.94%
String concatenation (Execution) 4.6±0.06µs 4.5±0.07µs -2.17%
String concatenation (Parser) 6.2±0.09µs 6.2±0.10µs 0.00%
String copy (Compiler) 1080.3±16.05ns 1142.8±16.26ns +5.79%
String copy (Execution) 4.1±0.07µs 4.1±0.07µs 0.00%
String copy (Parser) 4.7±0.04µs 4.7±0.09µs 0.00%
Symbols (Compiler) 771.5±13.13ns 790.5±11.03ns +2.46%
Symbols (Execution) 3.8±0.06µs 3.8±0.05µs 0.00%
Symbols (Parser) 3.6±0.06µs 3.6±0.06µs 0.00%

RageKnify
RageKnify approved these changes Mar 2, 2022
View reviewed changes
Copy link
Contributor

@RageKnify RageKnify left a comment

Choose a reason for hiding this comment

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

Nice, I'd noticed these tests were failing but couldn't understand the whole prototype vs constructor logic.
bors r+

bors bot pushed a commit that referenced this pull request Mar 2, 2022
@HalidOdat
%NativeError%.[[prototype]] should be Error constructor (#1883)
3c59905 
Before the `%NativeError%` objects (like `TypeError`, `ReferenceError`, etc) `[[prototype]]` field was set to `Function.prototype` but this is wrong it should be the `Error` constructor object itself.

This makes the  `%NativeError%`s 100% spec compliant :)
(except `AggregateError` because its not implemented)
@bors
Copy link

bors bot commented Mar 2, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title %NativeError%.[[prototype]] should be Error constructor [Merged by Bors] - %NativeError%.[[prototype]] should be Error constructor Mar 2, 2022
@bors bors bot closed this Mar 2, 2022
@bors bors bot deleted the fix/NativeError.__prototype__ branch March 2, 2022 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raskad raskad raskad approved these changes

@RageKnify RageKnify RageKnify approved these changes

@jedel1043 jedel1043 Awaiting requested review from jedel1043

@jasonwilliams jasonwilliams Awaiting requested review from jasonwilliams

@Razican Razican Awaiting requested review from Razican

Assignees
No one assigned
Labels
bug Something isn't working builtins PRs and Issues related to builtins/intrinsics
Projects
None yet
Milestone
v0.14.0
Development

Successfully merging this pull request may close these issues.
3 participants
@HalidOdat @RageKnify @raskad