Skip to content

Fix spans for lifted lambda idents #1854

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

Merged
merged 1 commit into from
Aug 16, 2024
Merged

Fix spans for lifted lambda idents #1854

merged 1 commit into from
Aug 16, 2024

Conversation

swernli
Copy link
Collaborator

@swernli swernli commented Aug 16, 2024

This change gets rid of the default span on lifted lambda names by setting it to the entire lamba. This ends up working well with error and lints that try to underline the lambda, which previously could end up highlighting the first character (sometimes of the wrong file).

Before:
image

After:
image

@swernli swernli requested a review from sezna August 16, 2024 04:50
Copy link

Benchmark for 0a78bbd

Click to view benchmark
Test Base PR %
Array append evaluation 338.5±4.00µs 338.8±2.91µs +0.09%
Array literal evaluation 185.9±1.08µs 169.6±5.36µs -8.77%
Array update evaluation 415.3±5.18µs 415.9±7.15µs +0.14%
Core + Standard library compilation 21.9±0.18ms 21.9±0.28ms 0.00%
Deutsch-Jozsa evaluation 5.0±0.11ms 4.9±0.06ms -2.00%
Large file parity evaluation 34.1±0.10ms 34.1±0.10ms 0.00%
Large input file compilation 13.7±0.15ms 14.0±0.42ms +2.19%
Large input file compilation (interpreter) 51.2±1.16ms 51.5±1.26ms +0.59%
Large nested iteration 32.9±0.76ms 32.7±0.46ms -0.61%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1557.0±28.92µs 1600.7±31.09µs +2.81%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.9±0.09ms 7.9±0.07ms 0.00%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1423.8±49.77µs 1460.5±35.41µs +2.58%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 27.7±0.22ms 28.0±0.34ms +1.08%
Teleport evaluation 93.6±3.70µs 94.4±4.75µs +0.85%

This change gets rid of the default span on lifted lambda names by setting it to the entire lamba. This ends up working well with error and lints that try to underline the lambda, which previously could end up highlighting the first character of the wrong file.
@swernli swernli force-pushed the swernli/lambda-span-fix branch from 53a8c81 to 740aea5 Compare August 16, 2024 05:04
Copy link

Benchmark for 6a610df

Click to view benchmark
Test Base PR %
Array append evaluation 337.9±4.51µs 335.3±2.58µs -0.77%
Array literal evaluation 186.7±2.76µs 186.5±0.90µs -0.11%
Array update evaluation 415.5±3.75µs 413.7±1.04µs -0.43%
Core + Standard library compilation 22.6±0.76ms 22.3±0.56ms -1.33%
Deutsch-Jozsa evaluation 5.0±0.05ms 5.0±0.06ms 0.00%
Large file parity evaluation 34.2±0.11ms 34.4±0.89ms +0.58%
Large input file compilation 14.2±0.45ms 14.0±0.31ms -1.41%
Large input file compilation (interpreter) 53.6±2.22ms 54.1±1.76ms +0.93%
Large nested iteration 32.8±0.24ms 32.5±0.19ms -0.91%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1571.6±61.79µs 1567.7±44.39µs -0.25%
Perform Runtime Capabilities Analysis (RCA) on large file sample 8.0±0.10ms 7.9±0.09ms -1.25%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1430.4±43.31µs 1432.1±54.08µs +0.12%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 28.0±0.42ms 27.8±0.39ms -0.71%
Teleport evaluation 94.2±4.30µs 94.3±3.63µs +0.11%

@swernli swernli added this pull request to the merge queue Aug 16, 2024
Merged via the queue into main with commit 6821ece Aug 16, 2024
19 checks passed
@swernli swernli deleted the swernli/lambda-span-fix branch August 16, 2024 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants