Skip to content

Conversation

petrochenkov
Copy link
Contributor

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 #62325
Previous iterations - #56647, #57155, #57915.

@rust-highfive
Copy link
Contributor

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

@bors

This comment has been minimized.

@petrochenkov
Copy link
Contributor Author

r? @Mark-Simulacrum

@Mark-Simulacrum
Copy link
Member

I think another test here that checks for us doing the right thing with an external crate, i.e., $crate when it resolves to foo vs. crate would be good -- but I'm not sure if we have one already.

With or without that, r=me

...but only if those tokens are printed from inside of AST pretty-printing.
Stop visiting AST to discover those contexts, just iterate through hygiene data instead
@petrochenkov
Copy link
Contributor Author

@bors r=Mark-Simulacrum

@bors
Copy link
Collaborator

bors commented Jul 9, 2019

📌 Commit 4cb67c0 has been approved by Mark-Simulacrum

@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 Jul 9, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 10, 2019
…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.
bors added a commit that referenced this pull request Jul 10, 2019
Rollup of 5 pull requests

Successful merges:

 - #61853 (Emit warning when trying to use PGO in conjunction with unwinding on …)
 - #62278 (Add Iterator::partition_in_place() and is_partitioned())
 - #62283 (Target::arch can take more than listed options)
 - #62393 (Fix pretty-printing of `$crate` (take 4))
 - #62474 (Prepare for LLVM 9 update)

Failed merges:

r? @ghost
@bors bors merged commit 4cb67c0 into rust-lang:master Jul 10, 2019
@bors
Copy link
Collaborator

bors commented Jul 10, 2019

⌛ Testing commit 4cb67c0 with merge 0324a2b...

@o01eg
Copy link
Contributor

o01eg commented Jul 10, 2019

I've got error with serde_derive after this PR:

error[E0433]: failed to resolve: unresolved import
  --> <::syn::token::Token macros>:37:55
   |
1  | / ( abstract ) => { crate :: token :: Abstract } ; ( as ) => {
2  | | crate :: token :: As } ; ( async ) => { crate :: token :: Async } ; ( auto )
3  | | => { crate :: token :: Auto } ; ( become ) => { crate :: token :: Become } ; (
4  | | box ) => { crate :: token :: Box } ; ( break ) => { crate :: token :: Break }
...  |
37 | | => { crate :: token :: Colon2 } ; ( , ) => { crate :: token :: Comma } ; ( / )
   | |                                                       ^^^^^
   | |                                                       |
   | |                                                       unresolved import
   | |                                                       help: a similar path exists: `syn::token`
...  |
54 | | ) => { crate :: token :: Tilde } ; ( _ ) => { crate :: token :: Underscore } ;
55 | | ( $ ) => { crate :: token :: Dollar } ;
   | |_______________________________________- in this expansion of `Token!`
   |
  ::: serde_derive/src/fragment.rs:58:18
   |
58 |                   <Token![,]>::default().to_tokens(out);
   |                    --------- in this macro invocation

error: aborting due to 12 previous errors

For more information about this error, try `rustc --explain E0433`.
error: Could not compile `serde_derive`.

@petrochenkov
Copy link
Contributor Author

@o01eg
Could you give some minimized / self-contained reproduction?
I'll look what happens.

bors added a commit that referenced this pull request Jul 11, 2019
pretty-print: Do not lose the `$crate` printing flag in `print_tt`

#62393 had this accidental mistake.

Fixes #62562
r? @Mark-Simulacrum
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 1, 2020
expand: Stop using nonterminals for passing tokens to attribute and derive macros

Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment).

Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later.

The cost is making pretty-printing of token streams less pretty.
Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway.

Unfortunately, this is not exactly the same thing as rust-lang#73102.
One more observable effect is how `$crate` is printed in the attribute input.
Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs).
This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed.

So, I think we should do this regardless, but we need to run crater first.
r? @Aaron1011
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 2, 2020
expand: Stop using nonterminals for passing tokens to attribute and derive macros

Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment).

Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later.

The cost is making pretty-printing of token streams less pretty.
Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway.

Unfortunately, this is not exactly the same thing as rust-lang#73102.
One more observable effect is how `$crate` is printed in the attribute input.
Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs).
This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed.

So, I think we should do this regardless, but we need to run crater first.
r? @Aaron1011
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 2, 2020
expand: Stop using nonterminals for passing tokens to attribute and derive macros

Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment).

Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later.

The cost is making pretty-printing of token streams less pretty.
Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway.

Unfortunately, this is not exactly the same thing as rust-lang#73102.
One more observable effect is how `$crate` is printed in the attribute input.
Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs).
This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed.

So, I think we should do this regardless, but we need to run crater first.
r? @Aaron1011
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 2, 2020
expand: Stop using nonterminals for passing tokens to attribute and derive macros

Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment).

Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later.

The cost is making pretty-printing of token streams less pretty.
Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway.

Unfortunately, this is not exactly the same thing as rust-lang#73102.
One more observable effect is how `$crate` is printed in the attribute input.
Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs).
This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed.

So, I think we should do this regardless, but we need to run crater first.
r? @Aaron1011
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 2, 2020
expand: Stop using nonterminals for passing tokens to attribute and derive macros

Make one more step towards fully token-based expansion and fix issues described in rust-lang#72545 (comment).

Now `struct S;` is passed to `foo!(struct S;)` and `#[foo] struct S;` in the same way - as a token stream `struct S ;`, rather than a single non-terminal token `NtItem` which is then broken into parts later.

The cost is making pretty-printing of token streams less pretty.
Some of the pretty-printing regressions will be recovered by keeping jointness with each token, which we will need to do anyway.

Unfortunately, this is not exactly the same thing as rust-lang#73102.
One more observable effect is how `$crate` is printed in the attribute input.
Inside `NtItem` was printed as `crate` or `that_crate`, now as a part of a token stream it's printed as `$crate` (there are good reasons for these differences, see rust-lang#62393 and related PRs).
This may break old proc macros (custom derives) written before the main portion of the proc macro API (macros 1.2) was stabilized, those macros did `input.to_string()` and reparsed the result, now that result can contain `$crate` which cannot be reparsed.

So, I think we should do this regardless, but we need to run crater first.
r? @Aaron1011
@petrochenkov petrochenkov deleted the notto-disu branch February 22, 2025 18:34
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$crate incorrectly substituted in attribute macro invocation containing bang macro invocation
6 participants