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

ToSource implementation is incorrect for signed integers #18091

Closed
apasel422 opened this issue Oct 16, 2014 · 2 comments
Closed

ToSource implementation is incorrect for signed integers #18091

apasel422 opened this issue Oct 16, 2014 · 2 comments
Labels
A-syntaxext Area: Syntax extensions E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@apasel422
Copy link
Contributor

It appears that the syntax::ext::quote::rt::ToSource implementation for signed integer types omits the sign.

test.rs:

extern crate syntax;

#[test]
fn test() {
    use syntax::ext::quote::rt::ToSource;

    let n = -5i32;
    let s = n.to_source();
    assert_eq!(s, "-5i32".to_string());
}

Output after compiling with rustc 0.13.0-nightly (40b244973 2014-10-14 23:22:20 +0000):

task 'test' failed at 'assertion failed: `(left == right) && (right == left)` (left: `5i32`, right: `-5i32`)'
@apasel422 apasel422 changed the title quote_expr macro discards sign from negative integer ToSource implementation is incorrect for signed integers Oct 16, 2014
@kmcallister kmcallister added A-syntaxext Area: Syntax extensions I-wrong E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Oct 16, 2014
@robinst
Copy link
Contributor

robinst commented Oct 19, 2014

The problem seems to be here:

ast_util::int_ty_to_string(st, Some(-(i as i64))).as_slice())

i is -5 as u64, so the above line passes Some(-((-5 as u64) as i64) to int_ty_to_string, which is Some(5). But then it doesn't add a - in front.

Where are the tests for this? Could try to write a fix.

bors added a commit that referenced this issue Oct 19, 2014
Fix for issue #18091 

The problem seems to be that `ast_util::int_ty_to_string` takes unsigned number, and no one adds `-` to result string. I've fixed it by putting `-` before result string using `format!`.

I've also added `test_signed_int_to_string()` to check if implementation is valid.
@apasel422
Copy link
Contributor Author

This was fixed by #18160.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-syntaxext Area: Syntax extensions E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

3 participants