Repository: dtolnay/syn (rust)

Issue:
Constructing a negative `LitInt` panics as unreachable
Minimal example:

```rust
use syn::LitInt;
use proc_macro2::Span;

fn main() {
    LitInt::new("-1", Span::call_site());
}
```

`cargo run` backtrace:

```text
thread 'main' panicked at 'internal error: entered unreachable code', /home/jhpratt/.cargo/registry/src/github.com-1ecc6299db9ec823/syn-1.0.13/src/lit.rs:1379:18
stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.40/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print_fmt
             at src/libstd/sys_common/backtrace.rs:84
   3: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
             at src/libstd/sys_common/backtrace.rs:61
   4: core::fmt::write
             at src/libcore/fmt/mod.rs:1025
   5: std::io::Write::write_fmt
             at src/libstd/io/mod.rs:1426
   6: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:65
   7: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:50
   8: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:193
   9: std::panicking::default_hook
             at src/libstd/panicking.rs:210
  10: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:475
  11: std::panicking::begin_panic
             at /rustc/0de96d37fbcc54978458c18f5067cd9817669bc8/src/libstd/panicking.rs:404
  12: syn::lit::value::to_literal
             at /home/jhpratt/.cargo/registry/src/github.com-1ecc6299db9ec823/syn-1.0.13/src/lit.rs:1379
  13: syn::lit::LitInt::new
             at /home/jhpratt/.cargo/registry/src/github.com-1ecc6299db9ec823/syn-1.0.13/src/lit.rs:407
  14: tmp::main
             at src/main.rs:5
  15: std::rt::lang_start::{{closure}}
             at /rustc/0de96d37fbcc54978458c18f5067cd9817669bc8/src/libstd/rt.rs:67
  16: std::rt::lang_start_internal::{{closure}}
             at src/libstd/rt.rs:52
  17: std::panicking::try::do_call
             at src/libstd/panicking.rs:292
  18: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:78
  19: std::panicking::try
             at src/libstd/panicking.rs:270
  20: std::panic::catch_unwind
             at src/libstd/panic.rs:394
  21: std::rt::lang_start_internal
             at src/libstd/rt.rs:51
  22: std::rt::lang_start
             at /rustc/0de96d37fbcc54978458c18f5067cd9817669bc8/src/libstd/rt.rs:67
  23: main
  24: __libc_start_main
  25: _start
```

Versions:
- syn: 1.0.13
- proc-macro2: 1.0.7

After taking a quick look at `lit.rs`, it looks like this should work. At the minimum, it definitely shouldn't reach an "unreachable!()" declaration.

I ran into this in real-world code, so it's certainly not unheard of to want to do something like this.

Pull request:
Support negative numeric literals
Fixes #748.

Patch:
diff --git a/src/lit.rs b/src/lit.rs
index abc4ec2f..0f271e48 100644
--- a/src/lit.rs
+++ b/src/lit.rs
@@ -403,18 +403,23 @@ impl LitChar {
 
 impl LitInt {
     pub fn new(repr: &str, span: Span) -> Self {
-        if let Some((digits, suffix)) = value::parse_lit_int(repr) {
-            let mut token = value::to_literal(repr);
-            token.set_span(span);
-            LitInt {
-                repr: Box::new(LitIntRepr {
-                    token,
-                    digits,
-                    suffix,
-                }),
-            }
-        } else {
-            panic!("Not an integer literal: `{}`", repr);
+        let (digits, suffix) = match value::parse_lit_int(repr) {
+            Some(parse) => parse,
+            None => panic!("Not an integer literal: `{}`", repr),
+        };
+
+        let mut token = match value::to_literal(repr, &digits, &suffix) {
+            Some(token) => token,
+            None => panic!("Unsupported integer literal: `{}`", repr),
+        };
+
+        token.set_span(span);
+        LitInt {
+            repr: Box::new(LitIntRepr {
+                token,
+                digits,
+                suffix,
+            }),
         }
     }
 
@@ -492,18 +497,23 @@ impl Display for LitInt {
 
 impl LitFloat {
     pub fn new(repr: &str, span: Span) -> Self {
-        if let Some((digits, suffix)) = value::parse_lit_float(repr) {
-            let mut token = value::to_literal(repr);
-            token.set_span(span);
-            LitFloat {
-                repr: Box::new(LitFloatRepr {
-                    token,
-                    digits,
-                    suffix,
-                }),
-            }
-        } else {
-            panic!("Not a float literal: `{}`", repr);
+        let (digits, suffix) = match value::parse_lit_float(repr) {
+            Some(parse) => parse,
+            None => panic!("Not a float literal: `{}`", repr),
+        };
+
+        let mut token = match value::to_literal(repr, &digits, &suffix) {
+            Some(token) => token,
+            None => panic!("Unsupported float literal: `{}`", repr),
+        };
+
+        token.set_span(span);
+        LitFloat {
+            repr: Box::new(LitFloatRepr {
+                token,
+                digits,
+                suffix,
+            }),
         }
     }
 
@@ -691,7 +701,9 @@ pub fn Lit(marker: lookahead::TokenMarker) -> Lit {
 #[cfg(feature = "parsing")]
 pub mod parsing {
     use super::*;
+    use crate::buffer::Cursor;
     use crate::parse::{Parse, ParseStream, Result};
+    use proc_macro2::Punct;
 
     impl Parse for Lit {
         fn parse(input: ParseStream) -> Result<Self> {
@@ -699,25 +711,73 @@ pub mod parsing {
                 if let Some((lit, rest)) = cursor.literal() {
                     return Ok((Lit::new(lit), rest));
                 }
-                while let Some((ident, rest)) = cursor.ident() {
-                    let value = if ident == "true" {
-                        true
-                    } else if ident == "false" {
-                        false
-                    } else {
-                        break;
-                    };
-                    let lit_bool = LitBool {
-                        value,
-                        span: ident.span(),
-                    };
-                    return Ok((Lit::Bool(lit_bool), rest));
+
+                if let Some((ident, rest)) = cursor.ident() {
+                    let value = ident == "true";
+                    if value || ident == "false" {
+                        let lit_bool = LitBool {
+                            value,
+                            span: ident.span(),
+                        };
+                        return Ok((Lit::Bool(lit_bool), rest));
+                    }
+                }
+
+                if let Some((punct, rest)) = cursor.punct() {
+                    if punct.as_char() == '-' {
+                        if let Some((lit, rest)) = parse_negative_lit(punct, rest) {
+                            return Ok((lit, rest));
+                        }
+                    }
                 }
+
                 Err(cursor.error("expected literal"))
             })
         }
     }
 
+    fn parse_negative_lit(neg: Punct, cursor: Cursor) -> Option<(Lit, Cursor)> {
+        let (lit, rest) = cursor.literal()?;
+
+        let mut span = neg.span();
+        span = span.join(lit.span()).unwrap_or(span);
+
+        let mut repr = lit.to_string();
+        repr.insert(0, '-');
+
+        if !(repr.ends_with("f32") || repr.ends_with("f64")) {
+            if let Some((digits, suffix)) = value::parse_lit_int(&repr) {
+                if let Some(mut token) = value::to_literal(&repr, &digits, &suffix) {
+                    token.set_span(span);
+                    return Some((
+                        Lit::Int(LitInt {
+                            repr: Box::new(LitIntRepr {
+                                token,
+                                digits,
+                                suffix,
+                            }),
+                        }),
+                        rest,
+                    ));
+                }
+            }
+        }
+
+        let (digits, suffix) = value::parse_lit_float(&repr)?;
+        let mut token = value::to_literal(&repr, &digits, &suffix)?;
+        token.set_span(span);
+        Some((
+            Lit::Float(LitFloat {
+                repr: Box::new(LitFloatRepr {
+                    token,
+                    digits,
+                    suffix,
+                }),
+            }),
+            rest,
+        ))
+    }
+
     impl Parse for LitStr {
         fn parse(input: ParseStream) -> Result<Self> {
             let head = input.fork();
@@ -1372,11 +1432,33 @@ mod value {
         }
     }
 
-    pub fn to_literal(s: &str) -> Literal {
-        let stream = s.parse::<TokenStream>().unwrap();
-        match stream.into_iter().next().unwrap() {
-            TokenTree::Literal(l) => l,
-            _ => unreachable!(),
+    pub fn to_literal(repr: &str, digits: &str, suffix: &str) -> Option<Literal> {
+        if repr.starts_with('-') {
+            if suffix == "f64" {
+                digits.parse().ok().map(Literal::f64_suffixed)
+            } else if suffix == "f32" {
+                digits.parse().ok().map(Literal::f32_suffixed)
+            } else if suffix == "i64" {
+                digits.parse().ok().map(Literal::i64_suffixed)
+            } else if suffix == "i32" {
+                digits.parse().ok().map(Literal::i32_suffixed)
+            } else if suffix == "i16" {
+                digits.parse().ok().map(Literal::i16_suffixed)
+            } else if suffix == "i8" {
+                digits.parse().ok().map(Literal::i8_suffixed)
+            } else if !suffix.is_empty() {
+                None
+            } else if digits.contains('.') {
+                digits.parse().ok().map(Literal::f64_unsuffixed)
+            } else {
+                digits.parse().ok().map(Literal::i64_unsuffixed)
+            }
+        } else {
+            let stream = repr.parse::<TokenStream>().unwrap();
+            match stream.into_iter().next().unwrap() {
+                TokenTree::Literal(l) => Some(l),
+                _ => unreachable!(),
+            }
         }
     }
 }


Test patch:
diff --git a/tests/test_attribute.rs b/tests/test_attribute.rs
index d77c0c09..e1b0142b 100644
--- a/tests/test_attribute.rs
+++ b/tests/test_attribute.rs
@@ -286,6 +286,48 @@ fn test_bool_lit() {
     "###);
 }
 
+#[test]
+fn test_negative_lit() {
+    let meta = test("#[form(min = -1, max = 200)]");
+
+    snapshot!(meta, @r###"
+    Meta::List {
+        path: Path {
+            segments: [
+                PathSegment {
+                    ident: "form",
+                    arguments: None,
+                },
+            ],
+        },
+        nested: [
+            Meta(Meta::NameValue {
+                path: Path {
+                    segments: [
+                        PathSegment {
+                            ident: "min",
+                            arguments: None,
+                        },
+                    ],
+                },
+                lit: -1,
+            }),
+            Meta(Meta::NameValue {
+                path: Path {
+                    segments: [
+                        PathSegment {
+                            ident: "max",
+                            arguments: None,
+                        },
+                    ],
+                },
+                lit: 200,
+            }),
+        ],
+    }
+    "###);
+}
+
 fn test(input: &str) -> Meta {
     let attrs = Attribute::parse_outer.parse_str(input).unwrap();
 
diff --git a/tests/test_lit.rs b/tests/test_lit.rs
index 0b300c4e..d4495ce4 100644
--- a/tests/test_lit.rs
+++ b/tests/test_lit.rs
@@ -1,9 +1,9 @@
 mod features;
 
-use proc_macro2::{TokenStream, TokenTree};
+use proc_macro2::{Span, TokenStream, TokenTree};
 use quote::ToTokens;
 use std::str::FromStr;
-use syn::Lit;
+use syn::{Lit, LitFloat, LitInt};
 
 fn lit(s: &str) -> Lit {
     match TokenStream::from_str(s)
@@ -182,3 +182,16 @@ fn floats() {
     test_float("1.0__3e-12", 1.03e-12, "");
     test_float("1.03e+12", 1.03e12, "");
 }
+
+#[test]
+fn negative() {
+    let span = Span::call_site();
+    assert_eq!("-1", LitInt::new("-1", span).to_string());
+    assert_eq!("-1i8", LitInt::new("-1i8", span).to_string());
+    assert_eq!("-1i16", LitInt::new("-1i16", span).to_string());
+    assert_eq!("-1i32", LitInt::new("-1i32", span).to_string());
+    assert_eq!("-1i64", LitInt::new("-1i64", span).to_string());
+    assert_eq!("-1.5", LitFloat::new("-1.5", span).to_string());
+    assert_eq!("-1.5f32", LitFloat::new("-1.5f32", span).to_string());
+    assert_eq!("-1.5f64", LitFloat::new("-1.5f64", span).to_string());
+}