Resolve $crates for pretty-printing at more appropriate time#57155
Resolve $crates for pretty-printing at more appropriate time#57155bors merged 2 commits intorust-lang:masterfrom
$crates for pretty-printing at more appropriate time#57155Conversation
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ |
|
📌 Commit e40d7d9 has been approved by |
bors
added
S-waiting-on-bors
Resolve `$crate`s for pretty-printing at more appropriate time Doing it in `BuildReducedGraphVisitor` wasn't a good idea, identifiers wasn't actually visited half of the time. As a result some `$crate`s weren't resolved and were therefore pretty-printed as `$crate` literally, which turns into two tokens during re-parsing of the pretty-printed text. Now we are visiting and resolving `$crate` identifiers in an item right before sending that item to a proc macro attribute or derive. Fixes #57089
|
☀️ Test successful - status-appveyor, status-travis |
|
@petrochenkov @dtolnay I noticed an issue with macro expansion with The issue started somewhere between nightlies I have an example here: https://gist.github.com/grovesNL/5d3ddc7b95c1939be26c64ba09f1ac2f (Original issue was reported in mozilla/cbindgen#272) |
|
It's not clear if there is a better alternative at the moment – it's definitely very useful to have the expanded macro source for use cases like cbindgen. I guess cbindgen could special case certain "known" macros in cbindgen (i.e. bitflags and others) or the downstream crates using cbindgen could manually expand macros. Both of these options are a bit problematic, because it would mean cbindgen is unable to work with most macros in general. |
|
Ok, I'll try to fix this specific case since it worked before. |
|
Thanks! Please note I also don’t mean to request a fix here if this use case isn’t supported, because it would inevitably break again at some point. I’m just trying to understand how we should go about making this work, even if it means reworking cbindgen heavily. #43364 discusses a long term fix a bit, but there doesn’t seem to be clear consensus yet. |
|
Fixed in #57915 |
Pretty print `$crate` as `crate` or `crate_name` in more cases So, people do parse output of `--pretty=expanded` (sigh), so covering only the legacy proc-macro case (like it was done in rust-lang#57155) is not enough. This PRs resolves all `$crate`s produced by macros, so they are all printed in the parseable form `$crate::foo` -> `crate::foo` or `crate_name::foo`. Fixes rust-lang#38016 (comment) Fixes rust-lang#57155 (comment)
Pretty print `$crate` as `crate` or `crate_name` in more cases So, people do parse output of `--pretty=expanded` (sigh), so covering only the legacy proc-macro case (like it was done in rust-lang#57155) is not enough. This PRs resolves all `$crate`s produced by macros, so they are all printed in the parseable form `$crate::foo` -> `crate::foo` or `crate_name::foo`. Fixes rust-lang#38016 (comment) Fixes rust-lang#57155 (comment)
…ulacrum Fix pretty-printing of `$crate` (take 4) Pretty-print `$crate` as `crate` or `crate_name` in unstructured tokens like `a $crate c` in `foo!(a $crate c)`, but only if those tokens are printed as a part of AST pretty-printing, rather than as a standalone token stream. Fixes rust-lang#62325 Previous iterations - rust-lang#56647, rust-lang#57155, rust-lang#57915.
5 participants
Doing it in
BuildReducedGraphVisitorwasn't a good idea, identifiers wasn't actually visited half of the time.As a result some
$crates weren't resolved and were therefore pretty-printed as$crateliterally, which turns into two tokens during re-parsing of the pretty-printed text.Now we are visiting and resolving
$crateidentifiers in an item right before sending that item to a proc macro attribute or derive.Fixes #57089