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

Negative float literals are not parsed after procedural macro #6028

Closed
cutsoy opened this issue Sep 17, 2020 · 2 comments · Fixed by #6158
Closed

Negative float literals are not parsed after procedural macro #6028

cutsoy opened this issue Sep 17, 2020 · 2 comments · Fixed by #6158
Labels
A-macro macro expansion

Comments

@cutsoy
Copy link
Contributor

cutsoy commented Sep 17, 2020

Disclaimer: I am not 100% sure whether this is a bug in RA or a missing validation in Rustc.

When writing -4.0f32 in Rust, it is usually parsed as two different tokens (a minus operator and a float literal).

But a procedural macro can also generate new tokens, including negative float literals:

#[proc_macro]
fn example_verbose(input: TokenStream) -> TokenStream {
    let literal = Literal::f32_suffixed(-42.0);
    quote! { #literal }
}

or even shorter

#[proc_macro]
fn example(input: TokenStream) -> TokenStream {
    let literal = -42.0f32;
    quote! { #literal }
}

Unfortunately, these currently cause RA to crash:

thread '<unnamed>' panicked at 'Fail to convert given literal Literal {
    text: "-42.0f32",
    id: TokenId(
        4294967295,
    ),
}', crates/mbe/src/subtree_source.rs:161:28

I use this as a temporary workaround, but it is clearly not ideal:

pub fn generate_f32(value: f32) -> TokenStream {
    let is_negative = value.is_sign_negative();

    if is_negative {
        let value = value.abs();
        quote! { - #value }
    } else {
        quote! { #value }
    }
}
@jonas-schievink jonas-schievink added the A-macro macro expansion label Sep 17, 2020
@cutsoy
Copy link
Contributor Author

cutsoy commented Oct 6, 2020

The work-around for this bug is a bit cumbersome, so I am hoping that providing more information might increase the chances of this getting fixed:

I think the problem is this:

  1. Within the context of a macro, proc_macro::Literal::f32_suffixed(-2.0) generates a new literal with text value "-2.0f32".
  2. RA tries to convert the resulting Literal("-2.0f32") token to an internal representation in convert_literal(l: &tt::Literal) -> TtToken, here. For this, it needs to know the kind of literal (e.g. float, char).
  3. Therefore, it calls the lexer to obtain the syntax kind of the first token it sees in the literal's text value. These calls are here and here.
  4. Finally, the lexer calls rustc_lexer::first_token(text) (here).
  5. Unfortunately, rustc_lexer considers the minus as a distinct (non-literal) token:
    fn main() {
        println!("Literal: {:?}", rustc_lexer::first_token("-2.0f32").kind);
        // >> Literal: Minus
    }
  6. We ascend back to convert_literal. This is where the problem finally comes to light:
    // fn convert_literal(l: &tt::Literal) -> TtToken
    let kind = lex_single_syntax_kind(&l.text)
        .map(|(kind, _error)| kind)       // kind = Minus
        .filter(|kind| kind.is_literal()) // kind.is_literal() = false
        // None
        .unwrap_or_else(|| panic!("Fail to convert given literal {:#?}", &l));

In a nutshell, the problem is first caused by the fact that:

  1. rustc_lexer considers -2.0 two distinct tokens ...
  2. but proc_macro's API generates a single token for -2.0.

There are three places where this might be fixed:

  1. In a future version, proc_macro's API should panic when Literal::f32_*(...) and friends are called with a negative value, here. (This wouldn't fix the problem per se, but it would at least add some validation.)
  2. In a future version, quote's implementation of ToTokens for f32 and friends should check if the given value is negative and use Punct::new(...) for the minus sign if necessary, here.
  3. In a future version, rust_analyzer/mbe should implement its own work-around by looking at the second token's syntax kind if the first token is a minus sign, here.

Unfortunately, none of 1-3 are currently implemented and as a result, importing a procedural macro that does quote! { -1.0 } now crashes RA server completely.

One more thing: in the proc_macro crate, the authors already acknowledge that negative literals might not survive a round-trip, which leads me to believe that this should probably be fixed in 3, rather than 1.

Any thoughts?

@cutsoy
Copy link
Contributor Author

cutsoy commented Oct 6, 2020

I submitted a similar issue to quote here. It might be easier to solve with a fix there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants