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

Wider callsite span #3466

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Wider callsite span #3466

merged 1 commit into from
Feb 21, 2024

Conversation

laurmaedje
Copy link
Member

@laurmaedje laurmaedje commented Feb 21, 2024

The compiler now uses the whole function call span instead of just the argument span as the callsite span. As a result, errors in a function are now annotated not just on the arguments, but on the whole function, e.g.:

#panic("hello")
 ^^^^^^^^^^^^^^
 panicked with: "hello"

instead of

#panic("hello")
      ^^^^^^^^^
      panicked with: "hello"

As a result, the missing arguments errors are now also on the full function call:

#rgb(0, 3)
 ^^^^^^^^^
 missing argument: blue component

instead of

#rgb(0, 3)
    ^^^^^^
    missing argument: blue component

One might consider that a bit less desirable than what we had before, but I think it's fine and actually better in some cases, e.g. one where a missing argument would have been annotated on a content body. We now have

#let foo(x, y) = {}
#foo[]
 ^^^^^
    missing argument: y

instead of

#let foo(x, y) = {}
#foo[]
    ^^
    missing argument: y

@Dherse
Copy link
Sponsor Collaborator

Dherse commented Feb 21, 2024

Interestingly, this is how #3307 works, which I assumed was a mistake in the VM🤔

@laurmaedje laurmaedje added this pull request to the merge queue Feb 21, 2024
@laurmaedje
Copy link
Member Author

Interestingly, this is how #3307 works, which I assumed was a mistake in the VM🤔

Well, I guess it would have been if the goal is to replicate the behaviour on main 1-1, but it's not anymore. :)

Merged via the queue into main with commit 56ecd6c Feb 21, 2024
8 checks passed
@laurmaedje laurmaedje deleted the wider-callsite-span branch February 21, 2024 14:17
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.

None yet

2 participants