Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Some hygiene doc improvements #146159

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

Open
camsteffen wants to merge 1 commit into rust-lang:master
base: master
Choose a base branch
Loading
from camsteffen:hygiene-docs

Conversation

Copy link
Contributor

@camsteffen camsteffen commented Sep 3, 2025
edited by Zalathar
Loading

Improve some doc comments around SyntaxContext, outer_expn and friends.

Based on discussion at #146100.

r? petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 3, 2025

This comment has been minimized.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2025
Copy link
Contributor Author

There's a couple things I'm still unclear on if you don't mind explaining.

In the dev guide section on "The Call-site Hierarchy", it says

macro bar($i: ident) { $i }
macro foo($i: ident) { $i }
foo!(bar!(baz));

For the baz AST node in the final output, the expansion-order hierarchy is ROOT -> id(foo) -> id(bar) -> baz, while the call-site hierarchy is ROOT -> baz.

But I did some testing and I think baz would just have the root SyntaxContext? So I don't know what to make of this. Also the example doesn't compile and I assume it should be macro foo($($t:tt)*)....

If I'm right about the above, is the call-site and the expansion-order hierarchies actually different? I can't think of an example where they would diverge.

I noticed when we mark spans for desugaring, we just use the root ExpnId for parent here. That seems like a potential bug.

@petrochenkov petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 5, 2025
Copy link
Contributor

But I did some testing and I think baz would just have the root SyntaxContext?

Yes, baz will have root context, because the token is not produced by any macro.

Also the example doesn't compile and I assume it should be macro foo($($t:tt)*)...

Yeah, there's a mistake there, it should be $($t:tt)*.

If I'm right about the above, is the call-site and the expansion-order hierarchies actually different? I can't think of an example where they would diverge.

Yes, they are different, and the example is supposed to demonstrate exactly that.
To get baz as a part of AST (not as a token) you need to expand foo and then bar, that's the expansion order hierarchy.
But the call site is still root, because it's token-oriented rather than AST-oriented.

I noticed when we mark spans for desugaring, we just use the root ExpnId for parent. That seems like a potential bug.

Using the root SyntaxContext (not ExpnId) as SyntaxContextData::parent for a desugaring is correct, because the "definition" of that desugaring does not come from any other macro, it's a built-in.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2025
/// The `Span` for the tokens in the previous macro expansion from which `self` was generated,
/// if any.
/// Returns the call-site span of the last macro expansion which produced this `Span`.
/// (see [`ExpnData::call_site`]). Returns `None` if this is not an expansion.
pub fn parent_callsite(self) -> Option<Span> {
Copy link
Contributor Author

@camsteffen camsteffen Sep 6, 2025

Choose a reason for hiding this comment

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

This name is a bit unfortunate since it does not use ExpnData::parent.

Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 6, 2025
Copy link
Contributor Author

Yes, baz will have root context, because the token is not produced by any macro.

I did some more testing and it seems the SyntaxContext for a Path depends on where the referenced thing is defined...which makes a a of sense.

Anyways, I think I've done enough damage here for now. I really appreciate your help with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petrochenkov petrochenkov petrochenkov left review comments

@fmease fmease fmease left review comments

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /