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] - Removing the panic in favour of an error result #1874

Closed
wants to merge 3 commits into from
Closed

[Merged by Bors] - Removing the panic in favour of an error result #1874

wants to merge 3 commits into from

Conversation

Razican
Copy link
Member

@Razican Razican commented Feb 27, 2022

This Pull Request is related to #1873.

It changes the following:

  • Removes the panic in case a label is not found. I think this should be an early syntax error, but at least we shouldn't panic while we fix the issue with the labels.

I think we should solve the issue with labeled statements for 0.15.

@Razican Razican added bug Something isn't working execution Issues or PRs related to code execution ast Issue surrounding the abstract syntax tree labels Feb 27, 2022
@Razican Razican added this to the v0.14.0 milestone Feb 27, 2022
@github-actions
Copy link

github-actions bot commented Feb 27, 2022 •
edited
Loading

Test262 conformance changes

VM implementation

Test result main count PR count difference
Total 88,342 88,342 0
Passed 42,933 42,933 0
Ignored 21,413 21,413 0
Failed 23,996 23,996 0
Panics 12 0 -12
Conformance 48.60% 48.60% 0.00%
Fixed panics (12): 
test/language/statements/do-while/S12.6.1_A4_T5.js [strict mode] (previously Panic)
test/language/statements/do-while/S12.6.1_A4_T5.js (previously Panic)
test/language/statements/while/S12.6.2_A4_T5.js [strict mode] (previously Panic)
test/language/statements/while/S12.6.2_A4_T5.js (previously Panic)
test/language/block-scope/leave/nested-block-let-declaration-only-shadows-outer-parameter-value-2.js [strict mode] (previously Panic)
test/language/block-scope/leave/nested-block-let-declaration-only-shadows-outer-parameter-value-2.js (previously Panic)
test/language/block-scope/leave/verify-context-in-labelled-block.js [strict mode] (previously Panic)
test/language/block-scope/leave/verify-context-in-labelled-block.js (previously Panic)
test/language/block-scope/leave/nested-block-let-declaration-only-shadows-outer-parameter-value-1.js [strict mode] (previously Panic)
test/language/block-scope/leave/nested-block-let-declaration-only-shadows-outer-parameter-value-1.js (previously Panic)
test/language/block-scope/leave/x-after-break-to-label.js [strict mode] (previously Panic)
test/language/block-scope/leave/x-after-break-to-label.js (previously Panic)

@Razican
Copy link
Member Author

Razican commented Feb 27, 2022

bors r+

@bors
Copy link

bors bot commented Feb 27, 2022

🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@github-actions
Copy link

Benchmark for a77fd13

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 529.4±20.27ns 526.4±25.10ns -0.57%
Arithmetic operations (Execution) 1947.3±94.02ns 2.0±0.10µs +2.71%
Arithmetic operations (Parser) 4.9±0.29µs 4.9±0.24µs 0.00%
Array access (Compiler) 1244.5±54.17ns 1284.4±60.40ns +3.21%
Array access (Execution) 9.7±0.46µs 9.8±0.38µs +1.03%
Array access (Parser) 10.6±0.51µs 10.6±0.46µs 0.00%
Array creation (Compiler) 1761.7±93.13ns 1829.4±87.93ns +3.84%
Array creation (Execution) 3.3±0.19ms 3.1±0.13ms -6.06%
Array creation (Parser) 12.3±0.63µs 11.9±0.58µs -3.25%
Array pop (Compiler) 3.7±0.15µs 3.7±0.18µs 0.00%
Array pop (Execution) 1414.8±70.79µs 1315.9±57.99µs -6.99%
Array pop (Parser) 126.5±6.51µs 126.7±6.75µs +0.16%
Boolean Object Access (Compiler) 1001.3±46.19ns 1007.2±41.77ns +0.59%
Boolean Object Access (Execution) 5.9±0.29µs 5.8±0.22µs -1.69%
Boolean Object Access (Parser) 13.4±0.75µs 13.0±0.49µs -2.99%
Clean js (Compiler) 3.4±0.15µs 3.3±0.16µs -2.94%
Clean js (Execution) 1114.5±64.66µs 1085.3±56.33µs -2.62%
Clean js (Parser) 25.5±1.23µs 26.0±1.14µs +1.96%
Create Realm 271.2±13.82ns 277.2±14.12ns +2.21%
Dynamic Object Property Access (Compiler) 1603.0±77.34ns 1662.3±84.58ns +3.70%
Dynamic Object Property Access (Execution) 7.1±0.32µs 6.8±0.34µs -4.23%
Dynamic Object Property Access (Parser) 9.5±0.44µs 9.3±0.53µs -2.11%
Fibonacci (Compiler) 2.2±0.09µs 2.3±0.13µs +4.55%
Fibonacci (Execution) 1764.4±84.44µs 1764.9±86.38µs +0.03%
Fibonacci (Parser) 15.2±0.66µs 14.6±0.70µs -3.95%
For loop (Compiler) 1947.0±111.89ns 1971.9±105.18ns +1.28%
For loop (Execution) 41.8±2.11µs 42.2±2.20µs +0.96%
For loop (Parser) 12.4±0.48µs 12.8±0.63µs +3.23%
Mini js (Compiler) 3.3±0.17µs 3.3±0.16µs 0.00%
Mini js (Execution) 1009.4±52.70µs 991.8±33.81µs -1.74%
Mini js (Parser) 22.3±0.93µs 22.6±1.05µs +1.35%
Number Object Access (Compiler) 942.9±42.17ns 987.9±52.14ns +4.77%
Number Object Access (Execution) 4.6±0.28µs 4.5±0.23µs -2.17%
Number Object Access (Parser) 10.1±0.54µs 10.2±0.61µs +0.99%
Object Creation (Compiler) 1388.2±75.46ns 1384.3±78.43ns -0.28%
Object Creation (Execution) 6.4±0.29µs 6.2±0.29µs -3.13%
Object Creation (Parser) 8.3±0.45µs 8.4±0.40µs +1.20%
RegExp (Compiler) 1654.8±86.83ns 1632.3±70.12ns -1.36%
RegExp (Execution) 11.8±0.48µs 11.9±0.47µs +0.85%
RegExp (Parser) 9.0±0.44µs 9.1±0.44µs +1.11%
RegExp Creation (Compiler) 1437.1±86.03ns 1419.8±60.83ns -1.20%
RegExp Creation (Execution) 9.2±0.41µs 9.0±0.47µs -2.17%
RegExp Creation (Parser) 7.5±0.36µs 7.6±0.39µs +1.33%
RegExp Literal (Compiler) 1629.7±83.99ns 1711.2±74.25ns +5.00%
RegExp Literal (Execution) 12.0±0.56µs 12.1±0.51µs +0.83%
RegExp Literal (Parser) 7.1±0.34µs 7.2±0.28µs +1.41%
RegExp Literal Creation (Compiler) 1393.7±67.71ns 1420.7±59.44ns +1.94%
RegExp Literal Creation (Execution) 9.4±0.46µs 8.9±0.42µs -5.32%
RegExp Literal Creation (Parser) 5.8±0.31µs 5.7±0.30µs -1.72%
Static Object Property Access (Compiler) 1463.5±64.72ns 1429.6±62.32ns -2.32%
Static Object Property Access (Execution) 6.6±0.31µs 6.3±0.26µs -4.55%
Static Object Property Access (Parser) 8.8±0.42µs 9.1±0.37µs +3.41%
String Object Access (Compiler) 1330.6±69.60ns 1361.3±59.66ns +2.31%
String Object Access (Execution) 7.4±0.27µs 7.8±0.37µs +5.41%
String Object Access (Parser) 13.4±0.48µs 13.2±0.83µs -1.49%
String comparison (Compiler) 2.2±0.15µs 2.2±0.13µs 0.00%
String comparison (Execution) 5.9±0.30µs 5.9±0.27µs 0.00%
String comparison (Parser) 9.8±0.49µs 10.1±0.51µs +3.06%
String concatenation (Compiler) 1623.0±77.87ns 1661.0±65.18ns +2.34%
String concatenation (Execution) 5.3±0.24µs 5.3±0.25µs 0.00%
String concatenation (Parser) 6.8±0.27µs 7.1±0.28µs +4.41%
String copy (Compiler) 1300.8±62.96ns 1286.6±49.94ns -1.09%
String copy (Execution) 5.0±0.23µs 4.7±0.25µs -6.00%
String copy (Parser) 4.9±0.25µs 5.3±0.27µs +8.16%
Symbols (Compiler) 916.8±52.55ns 950.0±44.15ns +3.62%
Symbols (Execution) 4.4±0.22µs 4.6±0.18µs +4.55%
Symbols (Parser) 3.7±0.16µs 3.9±0.20µs +5.41%

@github-actions
Copy link

Benchmark for 61712da

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 606.2±12.48ns 616.1±12.09ns +1.63%
Arithmetic operations (Execution) 2.3±0.05µs 2.3±0.03µs 0.00%
Arithmetic operations (Parser) 5.7±0.07µs 5.6±0.09µs -1.75%
Array access (Compiler) 1469.8±21.69ns 1502.6±22.63ns +2.23%
Array access (Execution) 11.6±0.15µs 11.7±0.11µs +0.86%
Array access (Parser) 12.2±0.36µs 12.5±0.23µs +2.46%
Array creation (Compiler) 2.1±0.04µs 2.1±0.01µs 0.00%
Array creation (Execution) 3.7±0.08ms 3.5±0.05ms -5.41%
Array creation (Parser) 14.0±0.20µs 14.1±0.40µs +0.71%
Array pop (Compiler) 4.2±0.07µs 4.3±0.05µs +2.38%
Array pop (Execution) 1626.8±29.89µs 1536.8±27.89µs -5.53%
Array pop (Parser) 143.6±2.98µs 145.2±2.96µs +1.11%
Boolean Object Access (Compiler) 1198.5±6.56ns 1203.9±9.41ns +0.45%
Boolean Object Access (Execution) 6.7±0.13µs 7.0±0.20µs +4.48%
Boolean Object Access (Parser) 15.0±0.39µs 15.1±0.31µs +0.67%
Clean js (Compiler) 3.8±0.08µs 3.9±0.06µs +2.63%
Clean js (Execution) 1290.8±14.71µs 1288.9±15.67µs -0.15%
Clean js (Parser) 29.8±0.75µs 30.7±0.32µs +3.02%
Create Realm 335.4±7.28ns 323.6±6.62ns -3.52%
Dynamic Object Property Access (Compiler) 1887.2±28.48ns 1906.3±28.27ns +1.01%
Dynamic Object Property Access (Execution) 7.9±0.10µs 8.1±0.12µs +2.53%
Dynamic Object Property Access (Parser) 10.8±0.22µs 11.1±0.14µs +2.78%
Fibonacci (Compiler) 2.6±0.04µs 2.6±0.05µs 0.00%
Fibonacci (Execution) 2.1±0.01ms 2.1±0.03ms 0.00%
Fibonacci (Parser) 17.1±0.27µs 17.0±0.32µs -0.58%
For loop (Compiler) 2.2±0.05µs 2.3±0.03µs +4.55%
For loop (Execution) 48.4±1.06µs 48.9±0.87µs +1.03%
For loop (Parser) 14.8±0.20µs 14.7±0.18µs -0.68%
Mini js (Compiler) 3.7±0.04µs 3.7±0.07µs 0.00%
Mini js (Execution) 1184.7±18.32µs 1154.4±36.12µs -2.56%
Mini js (Parser) 26.3±0.53µs 26.3±0.43µs 0.00%
Number Object Access (Compiler) 1077.8±21.05ns 1084.2±27.36ns +0.59%
Number Object Access (Execution) 5.2±0.11µs 5.4±0.12µs +3.85%
Number Object Access (Parser) 11.7±0.13µs 11.6±0.24µs -0.85%
Object Creation (Compiler) 1620.8±25.37ns 1581.1±43.20ns -2.45%
Object Creation (Execution) 7.2±0.03µs 7.1±0.16µs -1.39%
Object Creation (Parser) 9.5±0.15µs 9.8±0.14µs +3.16%
RegExp (Compiler) 1867.1±37.67ns 1912.5±39.33ns +2.43%
RegExp (Execution) 13.8±0.25µs 14.2±0.24µs +2.90%
RegExp (Parser) 10.5±0.16µs 10.8±0.11µs +2.86%
RegExp Creation (Compiler) 1645.1±27.12ns 1654.7±30.10ns +0.58%
RegExp Creation (Execution) 10.4±0.18µs 10.5±0.20µs +0.96%
RegExp Creation (Parser) 8.9±0.13µs 8.9±0.13µs 0.00%
RegExp Literal (Compiler) 1878.0±46.38ns 1914.7±33.74ns +1.95%
RegExp Literal (Execution) 14.0±0.24µs 14.1±0.35µs +0.71%
RegExp Literal (Parser) 8.5±0.11µs 8.6±0.10µs +1.18%
RegExp Literal Creation (Compiler) 1657.5±33.79ns 1648.1±28.02ns -0.57%
RegExp Literal Creation (Execution) 10.5±0.15µs 10.5±0.25µs 0.00%
RegExp Literal Creation (Parser) 6.7±0.10µs 6.7±0.12µs 0.00%
Static Object Property Access (Compiler) 1637.2±25.89ns 1639.1±27.54ns +0.12%
Static Object Property Access (Execution) 7.6±0.03µs 7.4±0.16µs -2.63%
Static Object Property Access (Parser) 10.3±0.17µs 10.5±0.19µs +1.94%
String Object Access (Compiler) 1600.4±14.05ns 1604.0±16.80ns +0.22%
String Object Access (Execution) 8.7±0.24µs 9.3±0.10µs +6.90%
String Object Access (Parser) 14.6±0.38µs 15.0±0.23µs +2.74%
String comparison (Compiler) 2.5±0.04µs 2.5±0.03µs 0.00%
String comparison (Execution) 6.8±0.12µs 6.9±0.12µs +1.47%
String comparison (Parser) 12.0±0.20µs 11.6±0.24µs -3.33%
String concatenation (Compiler) 1962.6±31.81ns 1964.1±30.77ns +0.08%
String concatenation (Execution) 6.1±0.13µs 6.2±0.11µs +1.64%
String concatenation (Parser) 8.1±0.10µs 8.0±0.12µs -1.23%
String copy (Compiler) 1519.4±23.70ns 1525.2±30.30ns +0.38%
String copy (Execution) 5.5±0.08µs 5.7±0.03µs +3.64%
String copy (Parser) 5.8±0.10µs 5.8±0.13µs 0.00%
Symbols (Compiler) 1047.5±29.42ns 1083.2±15.70ns +3.41%
Symbols (Execution) 5.3±0.10µs 5.5±0.04µs +3.77%
Symbols (Parser) 4.5±0.03µs 4.4±0.08µs -2.22%

@codecov
Copy link

codecov bot commented Feb 27, 2022 •
edited
Loading

Codecov Report

Merging #1874 (d8cd599) into main (60e2294) will decrease coverage by 0.01%.
The diff coverage is 25.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1874      +/-   ##
==========================================
- Coverage   46.67%   46.66%   -0.02%     
==========================================
  Files         204      204              
  Lines       16701    16707       +6     
==========================================
  Hits         7796     7796              
- Misses       8905     8911       +6
Impacted Files Coverage Δ
boa_engine/src/bytecompiler.rs 38.85% <25.00%> (+0.10%) ⬆️
boa_engine/src/vm/mod.rs 70.71% <0.00%> (-0.56%) ⬇️
boa_engine/src/value/display.rs 80.00% <0.00%> (-0.44%) ⬇️
boa_engine/src/vm/opcode.rs 42.85% <0.00%> (ø)
boa_engine/src/vm/code_block.rs 46.69% <0.00%> (ø)
boa_engine/src/value/equality.rs 90.00% <0.00%> (ø)

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 60e2294...d8cd599. Read the comment docs.

@github-actions
Copy link

Benchmark for 71825f1

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 510.4±1.64ns 505.5±0.99ns -0.96%
Arithmetic operations (Execution) 1974.5±1.81ns 1980.4±6.89ns +0.30%
Arithmetic operations (Parser) 5.2±0.00µs 5.2±0.01µs 0.00%
Array access (Compiler) 1248.1±2.79ns 1251.7±2.85ns +0.29%
Array access (Execution) 9.8±0.04µs 9.6±0.06µs -2.04%
Array access (Parser) 11.7±0.02µs 11.7±0.02µs 0.00%
Array creation (Compiler) 1834.4±5.40ns 1831.5±5.24ns -0.16%
Array creation (Execution) 3.2±0.01ms 3.1±0.01ms -3.13%
Array creation (Parser) 13.2±0.02µs 13.1±0.02µs -0.76%
Array pop (Compiler) 3.7±0.01µs 3.6±0.01µs -2.70%
Array pop (Execution) 1397.3±3.67µs 1353.9±3.26µs -3.11%
Array pop (Parser) 130.0±0.12µs 129.9±0.07µs -0.08%
Boolean Object Access (Compiler) 1096.8±4.26ns 1083.6±3.70ns -1.20%
Boolean Object Access (Execution) 6.0±0.06µs 5.7±0.05µs -5.00%
Boolean Object Access (Parser) 14.1±0.01µs 14.2±0.02µs +0.71%
Clean js (Compiler) 3.4±0.01µs 3.3±0.01µs -2.94%
Clean js (Execution) 1104.8±5.50µs 1079.9±6.07µs -2.25%
Clean js (Parser) 28.0±0.03µs 28.3±0.04µs +1.07%
Create Realm 272.4±0.41ns 268.9±0.33ns -1.28%
Dynamic Object Property Access (Compiler) 1583.7±40.08ns 1596.6±11.76ns +0.81%
Dynamic Object Property Access (Execution) 6.8±0.03µs 6.7±0.09µs -1.47%
Dynamic Object Property Access (Parser) 10.4±0.02µs 10.4±0.02µs 0.00%
Fibonacci (Compiler) 2.2±0.00µs 2.2±0.01µs 0.00%
Fibonacci (Execution) 1773.2±2.60µs 1753.0±2.53µs -1.14%
Fibonacci (Parser) 16.0±0.02µs 15.8±0.03µs -1.25%
For loop (Compiler) 1927.0±4.47ns 1917.7±10.03ns -0.48%
For loop (Execution) 43.3±0.16µs 41.9±0.07µs -3.23%
For loop (Parser) 13.5±0.03µs 13.7±0.08µs +1.48%
Mini js (Compiler) 3.2±0.01µs 3.2±0.01µs 0.00%
Mini js (Execution) 1017.7±6.75µs 992.6±12.36µs -2.47%
Mini js (Parser) 24.3±0.02µs 24.7±0.03µs +1.65%
Number Object Access (Compiler) 1019.9±4.84ns 1032.2±12.12ns +1.21%
Number Object Access (Execution) 4.6±0.02µs 4.5±0.02µs -2.17%
Number Object Access (Parser) 10.9±0.01µs 11.0±0.01µs +0.92%
Object Creation (Compiler) 1345.0±11.29ns 1345.4±2.32ns +0.03%
Object Creation (Execution) 6.1±0.02µs 6.0±0.02µs -1.64%
Object Creation (Parser) 9.1±0.01µs 9.0±0.01µs -1.10%
RegExp (Compiler) 1603.8±1.73ns 1595.8±16.18ns -0.50%
RegExp (Execution) 12.8±0.12µs 12.4±0.07µs -3.13%
RegExp (Parser) 10.0±0.03µs 9.9±0.01µs -1.00%
RegExp Creation (Compiler) 1364.9±5.70ns 1361.4±5.12ns -0.26%
RegExp Creation (Execution) 9.6±0.06µs 9.4±0.04µs -2.08%
RegExp Creation (Parser) 8.3±0.01µs 8.3±0.01µs 0.00%
RegExp Literal (Compiler) 1594.5±2.95ns 1585.2±2.68ns -0.58%
RegExp Literal (Execution) 12.8±0.03µs 12.5±0.04µs -2.34%
RegExp Literal (Parser) 8.1±0.01µs 8.0±0.01µs -1.23%
RegExp Literal Creation (Compiler) 1390.5±5.60ns 1375.6±5.72ns -1.07%
RegExp Literal Creation (Execution) 9.6±0.05µs 9.5±0.05µs -1.04%
RegExp Literal Creation (Parser) 6.3±0.01µs 6.3±0.01µs 0.00%
Static Object Property Access (Compiler) 1356.1±6.18ns 1363.4±7.39ns +0.54%
Static Object Property Access (Execution) 6.4±0.03µs 6.4±0.02µs 0.00%
Static Object Property Access (Parser) 9.9±0.01µs 9.8±0.03µs -1.01%
String Object Access (Compiler) 1462.8±8.52ns 1459.4±7.12ns -0.23%
String Object Access (Execution) 7.6±0.03µs 7.4±0.05µs -2.63%
String Object Access (Parser) 13.7±0.02µs 13.8±0.01µs +0.73%
String comparison (Compiler) 2.1±0.01µs 2.1±0.01µs 0.00%
String comparison (Execution) 5.9±0.01µs 5.8±0.02µs -1.69%
String comparison (Parser) 10.9±0.01µs 10.8±0.02µs -0.92%
String concatenation (Compiler) 1636.5±8.91ns 1621.0±2.19ns -0.95%
String concatenation (Execution) 5.3±0.02µs 5.3±0.02µs 0.00%
String concatenation (Parser) 7.4±0.02µs 7.4±0.02µs 0.00%
String copy (Compiler) 1307.3±2.93ns 1274.0±3.61ns -2.55%
String copy (Execution) 4.7±0.02µs 4.7±0.02µs 0.00%
String copy (Parser) 5.5±0.01µs 5.5±0.01µs 0.00%
Symbols (Compiler) 892.5±1.46ns 907.7±1.19ns +1.70%
Symbols (Execution) 4.5±0.01µs 4.4±0.02µs -2.22%
Symbols (Parser) 4.3±0.01µs 4.3±0.01µs 0.00%

@HalidOdat
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 27, 2022
@Razican
Removing the panic in favour of an error result (#1874)
1887b6a 
This Pull Request is related to #1873.

It changes the following:

- Removes the panic in case a label is not found. I think this should be an early syntax error, but at least we shouldn't panic while we fix the issue with the labels.

I think we should solve the issue with labeled statements for 0.15.
@bors
Copy link

bors bot commented Feb 27, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Removing the panic in favour of an error result [Merged by Bors] - Removing the panic in favour of an error result Feb 27, 2022
@bors bors bot closed this Feb 27, 2022
@bors bors bot deleted the panic_remove branch February 27, 2022 20:11
@HalidOdat HalidOdat mentioned this pull request Feb 27, 2022
Closed
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

@HalidOdat HalidOdat HalidOdat approved these changes

@jasonwilliams jasonwilliams Awaiting requested review from jasonwilliams

@RageKnify RageKnify Awaiting requested review from RageKnify

@jedel1043 jedel1043 Awaiting requested review from jedel1043

Assignees
No one assigned
Labels
ast Issue surrounding the abstract syntax tree bug Something isn't working execution Issues or PRs related to code execution
Projects
None yet
Milestone
v0.14.0
Development

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