Skip to content
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

Breaking behavior change for TokenStream containing trailing joint token #76399

Closed
dtolnay opened this issue Sep 6, 2020 · 2 comments · Fixed by #76556
Closed

Breaking behavior change for TokenStream containing trailing joint token #76399

dtolnay opened this issue Sep 6, 2020 · 2 comments · Fixed by #76556
Labels
A-proc-macros Area: Procedural macros P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Sep 6, 2020

Between nightly-2020-09-05 and nightly-2020-09-06, libproc_macro stopped preserving the jointness of a trailing joint punctuation token in a TokenStream (likely #76285; @matklad @petrochenkov).

Cargo.toml:

[package]
name = "repro"
version = "0.0.0"
edition = "2018"
[lib]
proc-macro = true

src/lib.rs:

use proc_macro::{Punct, Spacing, TokenStream, TokenTree};

#[proc_macro]
pub fn m(_input: TokenStream) -> TokenStream {
    eprintln!("{:#?}", TokenStream::from(TokenTree::Punct(Punct::new(':', Spacing::Joint))));
    TokenStream::new()
}

src/main.rs

repro::m!();
fn main() {}
$  cargo +nightly-2020-09-05 check
TokenStream [
    Punct {
        ch: ':',
        spacing: Joint,
        span: #7 bytes(0..12),
    },
]

$  cargo +nightly-2020-09-06 check
TokenStream [
    Punct {
        ch: ':',
        spacing: Alone,
        span: #7 bytes(0..12),
    },
]

This breaks some real-world code such as the following, though only with old versions of some libraries as far as I have found. I don't know which behavior above is correct but I would like it to be intentionally decided so that we know what behavior to implement in proc-macro2.

// [dependencies]
// serde = { version = "=1.0.113", features = ["derive"] }
// proc-macro2 = "=1.0.4"  # (works with >=1.0.5)

use serde::Serialize;

#[derive(Serialize)]
struct S {
    #[serde(skip_serializing_if = "Option::is_none")]
    field: Option<String>,
}

fails to compile since nightly-2020-09-06:

error: failed to parse path: "Option::is_none"
 --> src/main.rs:5:35
  |
5 |     #[serde(skip_serializing_if = "Option::is_none")]
  |                                   ^^^^^^^^^^^^^^^^^
@dtolnay dtolnay added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Sep 6, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 6, 2020
@jonas-schievink jonas-schievink added the A-proc-macros Area: Procedural macros label Sep 6, 2020
@Mark-Simulacrum
Copy link
Member

This has also broken perf.rust-lang.org's webrender-wrench, which relied on this behavior in one of the dependencies.

@matklad -- do you think a revert makes sense perhaps? Is there a patch expected soon-ish?

@Dylan-DPC-zz Dylan-DPC-zz added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 7, 2020
@jyn514 jyn514 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 7, 2020
@Dylan-DPC-zz
Copy link

Marking this as P-critical based on the wg-prioritization discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-proc-macros Area: Procedural macros P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants