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.

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

Clippy updates: add panics and etc. #3235

Merged
merged 9 commits into from
Sep 5, 2023
Merged

Clippy updates: add panics and etc. #3235

merged 9 commits into from
Sep 5, 2023

Conversation

nekevss
Copy link
Member

@nekevss nekevss commented Aug 24, 2023

This Pull Request is related to the failing CI on #3232.

It changes the following:

  • Adds the # Panics doc to the drop method.

EDIT: This kind of went a bit farther than I intended initially to just about all the lints. Comment is below.

@github-actions
Copy link

Test262 conformance changes

Test result main count PR count difference
Total 95,368 95,368 0
Passed 75,046 75,046 0
Ignored 19,302 19,302 0
Failed 1,020 1,020 0
Panics 0 0 0
Conformance 78.69% 78.69% 0.00%

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Patch coverage: 82.92% and no project coverage change.

Comparison is base (b47087c) 50.42% compared to head (645eb0d) 50.42%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3235   +/-   ##
=======================================
  Coverage   50.42%   50.42%           
=======================================
  Files         436      436           
  Lines       42549    42549           
=======================================
  Hits        21457    21457           
  Misses      21092    21092           
Files Changed Coverage Δ
boa_ast/src/position.rs 93.10% <ø> (ø)
boa_engine/src/builtins/map/ordered_map.rs 76.92% <ø> (ø)
boa_engine/src/builtins/set/ordered_set.rs 69.09% <ø> (ø)
boa_engine/src/lib.rs 74.02% <ø> (ø)
boa_engine/src/object/property_map.rs 68.95% <ø> (ø)
boa_engine/src/symbol.rs 70.27% <ø> (ø)
boa_engine/src/tagged.rs 90.47% <0.00%> (ø)
boa_gc/src/cell.rs 47.40% <ø> (ø)
boa_gc/src/internals/gc_box.rs 88.23% <ø> (ø)
boa_gc/src/trace.rs 91.66% <ø> (ø)
... and 19 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

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

Thanks!

@nekevss
Copy link
Member Author

nekevss commented Aug 24, 2023

Actually there's a lot more...clippy only feeds them one at a time apparently. Will hopefully update soon 😄

@nekevss nekevss changed the title Add the panic doc to Profiler's drop method Clippy Update: Add the panic doc to Profiler's drop method Aug 24, 2023
@nekevss
Copy link
Member Author

nekevss commented Aug 24, 2023

I updated a large amount of the lints in most the crates.

There were two lints that I didn't address and marked them as allow currently:

  1. There's a Send/Sync not implemented on Arc lint that is currently triggering on the Inner of JsSymbol
  2. There's A LOT of missing Panics docs in boa_engine (50+) so I ignored them for now (see below).

I wasn't sure if we should add panic docs on those functions or create an issue to remove the panics entirely and throw something of an ImplementationError or something of the sort.

@nekevss nekevss requested a review from a team August 24, 2023 16:51
@nekevss nekevss changed the title Clippy Update: Add the panic doc to Profiler's drop method Clippy updates: add panics and etc. Aug 24, 2023
@Razican
Copy link
Member

Razican commented Aug 25, 2023

Se should at least have an issue for each of those. Ideally removing panics where possible.

@jedel1043
Copy link
Member

Will merge this since it's holding back some dep updates, but I also agree that we should open issues for the pending tasks.

@jedel1043 jedel1043 enabled auto-merge August 26, 2023 20:45
@nekevss
Copy link
Member Author

nekevss commented Aug 26, 2023

I'll submit the issues 😄

@nekevss nekevss added this to the v0.18.0 milestone Aug 26, 2023
@jedel1043 jedel1043 disabled auto-merge August 26, 2023 23:01
@jedel1043 jedel1043 enabled auto-merge August 26, 2023 23:01
@nekevss nekevss added the Internal Category for changelog label Aug 26, 2023
@nekevss nekevss disabled auto-merge August 27, 2023 02:17
@nekevss nekevss enabled auto-merge August 27, 2023 02:17
@jedel1043 jedel1043 changed the base branch from main to async-run August 28, 2023 22:15
@jedel1043 jedel1043 changed the base branch from async-run to main August 28, 2023 22:15
@nekevss nekevss added benchmark Issues and PRs related to the benchmark subsystem. run-benchmark Label used to run banchmarks on PRs and removed benchmark Issues and PRs related to the benchmark subsystem. labels Aug 29, 2023
@github-actions
Copy link

Benchmark for a18da45

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 6.2±0.02ns 6.0±0.01ns -3.23%
Arithmetic operations (Execution) 435.8±1.16ns 438.0±0.27ns +0.50%
Arithmetic operations (Parser) 8.9±0.15µs 9.0±0.08µs +1.12%
Array access (Compiler) 6.2±0.02ns 6.0±0.01ns -3.23%
Array access (Execution) 6.2±13.19µs 6.8±15.10µs +9.68%
Array access (Parser) 19.5±36.70µs 15.8±0.12µs -18.97%
Array creation (Compiler) 6.2±0.17ns 6.0±0.01ns -3.23%
Array creation (Execution) 1260.1±4984.19µs 890.6±1523.49µs -29.32%
Array creation (Parser) 19.2±0.47µs 24.5±50.15µs +27.60%
Array pop (Compiler) 6.2±0.01ns 6.0±0.00ns -3.23%
Array pop (Execution) 284.0±306.03µs 291.9±298.11µs +2.78%
Array pop (Parser) 177.3±4.45µs 176.9±3.26µs -0.23%
Boolean Object Access (Compiler) 6.2±0.01ns 6.0±0.01ns -3.23%
Boolean Object Access (Execution) 5.7±11.14µs 5.7±10.52µs 0.00%
Boolean Object Access (Parser) 18.5±0.27µs 18.6±1.08µs +0.54%
Clean js (Compiler) 6.2±0.00ns 6.6±0.01ns +6.45%
Clean js (Execution) 559.2±511.57µs 573.3±534.97µs +2.52%
Clean js (Parser) 40.4±0.95µs 41.0±0.33µs +1.49%
Create Realm 650.2±1845.41µs 691.3±1877.84µs +6.32%
Dynamic Object Property Access (Compiler) 6.2±0.02ns 6.0±0.03ns -3.23%
Dynamic Object Property Access (Execution) 3.5±9.33µs 3.6±8.04µs +2.86%
Dynamic Object Property Access (Parser) 13.8±0.27µs 13.5±0.40µs -2.17%
Fibonacci (Compiler) 6.2±0.02ns 6.0±0.02ns -3.23%
Fibonacci (Execution) 883.8±1600.32µs 762.5±1475.75µs -13.72%
Fibonacci (Parser) 21.6±0.81µs 22.5±0.26µs +4.17%
For loop (Compiler) 6.3±0.18ns 6.0±0.01ns -4.76%
For loop (Execution) 28.2±73.80µs 39.0±147.09µs +38.30%
For loop (Parser) 19.7±0.83µs 20.4±0.33µs +3.55%
Mini js (Compiler) 6.2±0.04ns 6.0±0.02ns -3.23%
Mini js (Execution) 483.6±2.24µs 503.1±2.49µs +4.03%
Mini js (Parser) 37.0±2.07µs 36.2±0.93µs -2.16%
Number Object Access (Compiler) 6.2±0.02ns 6.0±0.00ns -3.23%
Number Object Access (Execution) 4.9±9.61µs 4.2±6.13µs -14.29%
Number Object Access (Parser) 14.8±0.81µs 15.0±1.81µs +1.35%
Object Creation (Compiler) 6.2±0.05ns 6.0±0.00ns -3.23%
Object Creation (Execution) 3.4±10.36µs 3.9±12.43µs +14.71%
Object Creation (Parser) 11.7±0.26µs 12.2±0.34µs +4.27%
RegExp (Compiler) 6.2±0.01ns 6.0±0.01ns -3.23%
RegExp (Execution) 23.9±112.22µs 15.0±22.31µs -37.24%
RegExp (Parser) 12.8±0.20µs 13.0±0.11µs +1.56%
RegExp Creation (Compiler) 6.2±0.03ns 6.0±0.01ns -3.23%
RegExp Creation (Execution) 8.2±12.68µs 21.2±144.52µs +158.54%
RegExp Creation (Parser) 10.9±0.59µs 10.9±0.11µs 0.00%
RegExp Literal (Compiler) 6.2±0.08ns 6.0±0.01ns -3.23%
RegExp Literal (Execution) 14.7±23.53µs 13.0±17.34µs -11.56%
RegExp Literal (Parser) 14.5±0.31µs 14.6±0.14µs +0.69%
RegExp Literal Creation (Compiler) 6.2±0.00ns 6.0±0.00ns -3.23%
RegExp Literal Creation (Execution) 8.8±20.01µs 8.5±13.47µs -3.41%
RegExp Literal Creation (Parser) 12.4±0.20µs 12.3±0.25µs -0.81%
Static Object Property Access (Compiler) 6.2±0.02ns 6.0±0.01ns -3.23%
Static Object Property Access (Execution) 3.8±8.80µs 3.0±8.25µs -21.05%
Static Object Property Access (Parser) 19.5±64.47µs 13.1±0.54µs -32.82%
String Object Access (Compiler) 6.2±0.01ns 6.0±0.01ns -3.23%
String Object Access (Execution) 7.4±10.90µs 7.1±9.91µs -4.05%
String Object Access (Parser) 17.9±0.14µs 18.3±0.96µs +2.23%
String comparison (Compiler) 6.2±0.01ns 6.0±0.00ns -3.23%
String comparison (Execution) 3.1±7.28µs 4.1±12.73µs +32.26%
String comparison (Parser) 16.0±0.32µs 16.0±0.28µs 0.00%
String concatenation (Compiler) 6.2±0.00ns 6.0±0.00ns -3.23%
String concatenation (Execution) 3.1±8.38µs 2.6±6.45µs -16.13%
String concatenation (Parser) 10.5±0.26µs 10.8±0.10µs +2.86%
String copy (Compiler) 6.2±0.01ns 6.0±0.01ns -3.23%
String copy (Execution) 2.3±5.70µs 2.8±6.82µs +21.74%
String copy (Parser) 8.4±1.71µs 7.9±0.18µs -5.95%
Symbols (Compiler) 6.2±0.01ns 6.0±0.04ns -3.23%
Symbols (Execution) 2.0±0.04µs 1977.0±28.56ns -1.15%
Symbols (Parser) 5.8±0.17µs 5.8±0.17µs 0.00%

@HalidOdat HalidOdat disabled auto-merge September 3, 2023 16:49
@HalidOdat HalidOdat enabled auto-merge September 3, 2023 16:49
@HalidOdat HalidOdat disabled auto-merge September 3, 2023 17:32
@HalidOdat HalidOdat enabled auto-merge September 3, 2023 17:33
@jedel1043 jedel1043 disabled auto-merge September 5, 2023 05:12
@jedel1043 jedel1043 enabled auto-merge September 5, 2023 05:13
@github-actions
Copy link

github-actions bot commented Sep 5, 2023

Benchmark for 45000d8

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 7.4±0.05ns 7.2±0.06ns -2.70%
Arithmetic operations (Execution) 514.6±8.10ns 515.7±7.52ns +0.21%
Arithmetic operations (Parser) 52.4±413.64µs 45.3±336.14µs -13.55%
Array access (Compiler) 7.4±0.06ns 7.2±0.10ns -2.70%
Array access (Execution) 8.3±22.95µs 6.6±11.10µs -20.48%
Array access (Parser) 17.0±0.32µs 17.6±0.18µs +3.53%
Array creation (Compiler) 7.3±0.11ns 7.1±0.09ns -2.74%
Array creation (Execution) 947.1±1255.52µs 1035.8±1733.41µs +9.37%
Array creation (Parser) 21.5±0.37µs 21.6±0.33µs +0.47%
Array pop (Compiler) 7.9±0.10ns 7.7±0.10ns -2.53%
Array pop (Execution) 334.1±275.80µs 339.3±347.88µs +1.56%
Array pop (Parser) 207.0±13.02µs 207.3±22.89µs +0.14%
Boolean Object Access (Compiler) 7.4±0.10ns 7.1±0.11ns -4.05%
Boolean Object Access (Execution) 7.5±16.68µs 5.7±8.04µs -24.00%
Boolean Object Access (Parser) 19.8±0.40µs 19.8±0.21µs 0.00%
Clean js (Compiler) 7.4±0.07ns 7.1±0.12ns -4.05%
Clean js (Execution) 638.7±393.17µs 708.1±703.64µs +10.87%
Clean js (Parser) 43.7±1.05µs 45.0±1.00µs +2.97%
Create Realm 684.1±1719.62µs 667.3±1679.60µs -2.46%
Dynamic Object Property Access (Compiler) 8.0±0.07ns 7.1±0.09ns -11.25%
Dynamic Object Property Access (Execution) 3.9±7.87µs 3.3±7.52µs -15.38%
Dynamic Object Property Access (Parser) 34.4±192.21µs 25.6±102.73µs -25.58%
Fibonacci (Compiler) 7.4±0.05ns 7.1±0.10ns -4.05%
Fibonacci (Execution) 999.2±2118.93µs 879.9±1994.27µs -11.94%
Fibonacci (Parser) 23.7±0.65µs 25.1±0.43µs +5.91%
For loop (Compiler) 7.4±0.10ns 7.1±0.09ns -4.05%
For loop (Execution) 27.9±64.21µs 30.6±69.83µs +9.68%
For loop (Parser) 22.2±0.92µs 22.6±0.53µs +1.80%
Mini js (Compiler) 7.4±0.09ns 7.1±0.14ns -4.05%
Mini js (Execution) 571.9±6.74µs 592.3±5.23µs +3.57%
Mini js (Parser) 38.9±1.04µs 39.6±0.69µs +1.80%
Number Object Access (Compiler) 7.4±0.10ns 7.2±0.08ns -2.70%
Number Object Access (Execution) 4.3±5.19µs 4.4±5.72µs +2.33%
Number Object Access (Parser) 15.7±0.35µs 15.9±0.21µs +1.27%
Object Creation (Compiler) 7.4±0.08ns 7.1±0.09ns -4.05%
Object Creation (Execution) 3.1±7.62µs 3.2±8.12µs +3.23%
Object Creation (Parser) 12.9±0.58µs 13.5±0.19µs +4.65%
RegExp (Compiler) 7.4±0.09ns 7.1±0.12ns -4.05%
RegExp (Execution) 15.9±19.97µs 15.5±17.23µs -2.52%
RegExp (Parser) 14.0±0.31µs 14.5±0.19µs +3.57%
RegExp Creation (Compiler) 7.4±0.07ns 7.7±0.10ns +4.05%
RegExp Creation (Execution) 7.6±8.74µs 8.7±9.80µs +14.47%
RegExp Creation (Parser) 11.9±0.23µs 12.0±0.26µs +0.84%
RegExp Literal (Compiler) 7.4±0.07ns 7.1±0.11ns -4.05%
RegExp Literal (Execution) 13.6±17.50µs 15.4±17.63µs +13.24%
RegExp Literal (Parser) 16.2±0.31µs 16.5±0.18µs +1.85%
RegExp Literal Creation (Compiler) 7.4±0.05ns 7.1±0.12ns -4.05%
RegExp Literal Creation (Execution) 9.5±18.76µs 7.6±8.61µs -20.00%
RegExp Literal Creation (Parser) 13.7±0.31µs 13.6±0.37µs -0.73%
Static Object Property Access (Compiler) 7.4±0.09ns 7.1±0.09ns -4.05%
Static Object Property Access (Execution) 4.3±12.06µs 3.6±7.51µs -16.28%
Static Object Property Access (Parser) 13.6±0.45µs 14.3±0.17µs +5.15%
String Object Access (Compiler) 7.4±0.05ns 7.2±0.05ns -2.70%
String Object Access (Execution) 8.6±14.04µs 10.4±24.22µs +20.93%
String Object Access (Parser) 20.1±0.20µs 19.9±0.14µs -1.00%
String comparison (Compiler) 7.3±0.12ns 7.2±0.07ns -1.37%
String comparison (Execution) 2.9±5.78µs 3.5±6.00µs +20.69%
String comparison (Parser) 17.5±0.36µs 17.2±0.48µs -1.71%
String concatenation (Compiler) 7.4±0.13ns 7.2±0.11ns -2.70%
String concatenation (Execution) 3.2±8.79µs 4.1±15.35µs +28.12%
String concatenation (Parser) 11.9±0.27µs 11.9±0.35µs 0.00%
String copy (Compiler) 7.3±0.11ns 7.2±0.06ns -1.37%
String copy (Execution) 3.0±9.43µs 2.7±6.19µs -10.00%
String copy (Parser) 8.6±0.29µs 8.6±0.14µs 0.00%
Symbols (Compiler) 7.4±0.06ns 7.1±0.13ns -4.05%
Symbols (Execution) 2.9±8.17µs 3.0±8.44µs +3.45%
Symbols (Parser) 6.2±0.17µs 6.3±0.17µs +1.61%

@jedel1043 jedel1043 added this pull request to the merge queue Sep 5, 2023
Merged via the queue into main with commit f92e748 Sep 5, 2023
@jedel1043 jedel1043 deleted the add-panic-doc branch September 5, 2023 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Category for changelog run-benchmark Label used to run banchmarks on PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants