Skip to content

Conversation

camsteffen
Copy link
Contributor

@camsteffen camsteffen commented Sep 3, 2025

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
@rust-log-analyzer

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
@camsteffen
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
@petrochenkov
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

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.

@camsteffen
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
@camsteffen
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.

@petrochenkov
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Sep 13, 2025

📌 Commit 31b3915 has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 13, 2025
bors added a commit that referenced this pull request Sep 13, 2025
Rollup of 8 pull requests

Successful merges:

 - #113095 (Document `become` keyword)
 - #146159 (Some hygiene doc improvements)
 - #146171 (tidy: check that error messages don't start with a capitalized letter)
 - #146419 (Update the arm-* and aarch64-* platform docs.)
 - #146473 (Revert "Constify SystemTime methods")
 - #146506 (Fix small typo in check-cfg.md)
 - #146517 (fix Condvar::wait_timeout docs)
 - #146521 (document `core::ffi::VaArgSafe`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4ee860f into rust-lang:master Sep 14, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Sep 14, 2025
rust-timer added a commit that referenced this pull request Sep 14, 2025
Rollup merge of #146159 - camsteffen:hygiene-docs, r=petrochenkov

Some hygiene doc improvements

Improve some doc comments around SyntaxContext, outer_expn and friends.

Based on discussion at #146100.

r? petrochenkov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants