Fix pretty-printing of $crate (take 4) by petrochenkov · Pull Request #62393 · rust-lang/rust (original) (raw)

petrochenkov

Pretty-print $crate as crate or crate_name in unstructured tokens like a <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mi>c</mi><mi>r</mi><mi>a</mi><mi>t</mi><mi>e</mi><mi>c</mi><mi mathvariant="normal">‘</mi><mi>i</mi><mi>n</mi><mi mathvariant="normal">‘</mi><mi>f</mi><mi>o</mi><mi>o</mi><mo stretchy="false">!</mo><mo stretchy="false">(</mo><mi>a</mi></mrow><annotation encoding="application/x-tex">crate c in foo!(a </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mord mathnormal" style="margin-right:0.02778em;">cr</span><span class="mord mathnormal">a</span><span class="mord mathnormal">t</span><span class="mord mathnormal">ec</span><span class="mord">‘</span><span class="mord mathnormal">in</span><span class="mord">‘</span><span class="mord mathnormal" style="margin-right:0.10764em;">f</span><span class="mord mathnormal">oo</span><span class="mclose">!</span><span class="mopen">(</span><span class="mord mathnormal">a</span></span></span></span>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

r? @pnkfelix

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

@bors

This comment has been minimized.

@petrochenkov

@Mark-Simulacrum

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

@petrochenkov

@petrochenkov

...but only if those tokens are printed from inside of AST pretty-printing.

@petrochenkov

Stop visiting AST to discover those contexts, just iterate through hygiene data instead

@petrochenkov

@petrochenkov

@bors

📌 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

@Centril

…ulacrum

Fix pretty-printing of $crate (take 4)

Pretty-print $crate as crate or crate_name in unstructured tokens like a <span class="katex"><span class="katex-mathml"><math xmlns="http://www.w3.org/1998/Math/MathML"><semantics><mrow><mi>c</mi><mi>r</mi><mi>a</mi><mi>t</mi><mi>e</mi><mi>c</mi><mi mathvariant="normal">‘</mi><mi>i</mi><mi>n</mi><mi mathvariant="normal">‘</mi><mi>f</mi><mi>o</mi><mi>o</mi><mo stretchy="false">!</mo><mo stretchy="false">(</mo><mi>a</mi></mrow><annotation encoding="application/x-tex">crate c in foo!(a </annotation></semantics></math></span><span class="katex-html" aria-hidden="true"><span class="base"><span class="strut" style="height:1em;vertical-align:-0.25em;"></span><span class="mord mathnormal" style="margin-right:0.02778em;">cr</span><span class="mord mathnormal">a</span><span class="mord mathnormal">t</span><span class="mord mathnormal">ec</span><span class="mord">‘</span><span class="mord mathnormal">in</span><span class="mord">‘</span><span class="mord mathnormal" style="margin-right:0.10764em;">f</span><span class="mord mathnormal">oo</span><span class="mclose">!</span><span class="mopen">(</span><span class="mord mathnormal">a</span></span></span></span>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

@bors

Rollup of 5 pull requests

Successful merges:

Failed merges:

r? @ghost

@bors

@o01eg

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

@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

@bors

@hawkw hawkw mentioned this pull request

Aug 1, 2019

This was referenced

Aug 25, 2019

Manishearth added a commit to Manishearth/rust that referenced this pull request

Jul 1, 2020

@Manishearth

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

@Manishearth

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

@Manishearth

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

@Manishearth

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

@Manishearth

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