-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Some hygiene doc improvements #146100
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
Some hygiene doc improvements #146100
Conversation
r? @SparrowLii rustbot has assigned @SparrowLii. Use |
Some changes occurred to the CTFE machinery Some changes occurred in src/tools/clippy cc @rust-lang/clippy Some changes occurred in compiler/rustc_ast_lowering/src/format.rs cc @m-ou-se The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes. cc @BoxyUwU, @jieyouxu, @Kobzol, @tshepang Some changes occurred in check-cfg diagnostics cc @Urgau Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred in coverage instrumentation. cc @Zalathar |
r? petrochenkov |
|
Couldn't it also be the innermost or anything in between? I think I see where you're coming from now. "outer" means "this expansion which might have child expansions". It confused me because I thought it meant "get the parent expansion relative to this expansion". But that is
To me it just says "give me the expansion of this span" without making any implications about parent or child expansions. Anyways, this could just be me. |
You can call it the innermost if you count from the opposite side, but certainly not something in between.
A span (or rather |
I see what happened. I was thinking of SyntaxContext as being an ID for just the outer(?) expansion rather than representing the whole chain. With that shift, all the methods on SyntaxContext make much more sense now.
I think this is part of what stalled my understanding. I think of I can change this PR to just add some docs. Or what do you think of renaming to |
Let's just keep the current naming to avoid churn. |
683c2dd
to
13d7600
Compare
@rustbot ready |
This PR has changed enough from its original scope that I would suggest closing it, and creating a new PR for the doc updates (with a link back to this one). Otherwise, the change ends up being very confusing. |
Ok, see #146159 |
The job Click to see the possible cause of the failure (guessed by this bot)
|
This naming has always confused me. I'm not sure what "outer" is supposed to mean. Zulip thread.Improve some doc comments around SyntaxContext, outer_expn and friends.