Skip to content

Conversation

@Shourya742
Copy link
Contributor

This PR extends the proc-macro bidirectional protocol to support Span::file() and Span::local_file().

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 31, 2025
@Veykril Veykril self-assigned this Dec 31, 2025
Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

Why do local file name and file name differ for us here? I'd expect both of the functions to return the same results for us, reading the corresponding function docs https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.file and https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.local_file

@Shourya742
Copy link
Contributor Author

Why do local file name and file name differ for us here? I'd expect both of the functions to return the same results for us, reading the corresponding function docs https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.file and https://doc.rust-lang.org/stable/proc_macro/struct.Span.html#method.local_file

Ahh, right, fetch the local filename first, and if that doesn’t exist, fall back to manually constructing it. I was too focused on the display-side logic for the filename and totally missed the obvious. Thanks for pointing it out!

@Shourya742 Shourya742 requested a review from Veykril January 2, 2026 19:57
@Veykril
Copy link
Member

Veykril commented Jan 2, 2026

No my point was more that for rust-analyzer, both functions will effectively return the same info. rustc has some extra stuff like path remapping and what not that are only handled in one of the two functions, we don't though.

Also ntoably, both functions are supposed to return full paths, not just file names.

@Shourya742
Copy link
Contributor Author

Shourya742 commented Jan 2, 2026

No my point was more that for rust-analyzer, both functions will effectively return the same info. rustc has some extra stuff like path remapping and what not that are only handled in one of the two functions, we don't though.

Also ntoably, both functions are supposed to return full paths, not just file names.

Are you suggesting we don't return a file name, when absolute path doesn't exists? because currently, we first try to get the path, and if it doesn't exist, just return the filename. And considering filename method is just for display purpose, so that should be kinda ok? Otherwise, we gonna return empty string, which won't be much informative.

@Shourya742
Copy link
Contributor Author

No my point was more that for rust-analyzer, both functions will effectively return the same info. rustc has some extra stuff like path remapping and what not that are only handled in one of the two functions, we don't though.

Also ntoably, both functions are supposed to return full paths, not just file names.

After some more thought, the suggestion clicked. I’ve aligned filename and localfilename so they now emit the same info. Thanks for the suggestion, it helped me understand this part better.

@Veykril
Copy link
Member

Veykril commented Jan 3, 2026

There is still some slight confusion here I think, your implementation returns the file name only, but it should really return the entire file path.

That is, these are also misnamed as they are about the full path, not just the file name.

@Shourya742 Shourya742 force-pushed the 2025-12-31-add-more-proc-macro-bidirectional-flow branch from 20bf54f to 1684686 Compare January 3, 2026 09:38
@Shourya742 Shourya742 force-pushed the 2025-12-31-add-more-proc-macro-bidirectional-flow branch from 1684686 to 26b3c82 Compare January 3, 2026 09:44
@Shourya742
Copy link
Contributor Author

There is still some slight confusion here I think, your implementation returns the file name only, but it should really return the entire file path.

That is, these are also misnamed as they are about the full path, not just the file name.

Shoot, thanks for mentioning that. Ah!!! my bad... so sorry, I completely mixed it with filename for some reason. Updated.

@Veykril Veykril added this pull request to the merge queue Jan 3, 2026
Merged via the queue into rust-lang:master with commit 036c71a Jan 3, 2026
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 3, 2026
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.

4 participants