-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix failing type interference for floating point literal #1604
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
Conversation
Just a quick note: you don't need to go that route to test this. You can simply use the "Show syntax tree" command in VS Code. |
The lexer decides that 42f64 is an integer literal here: https://github.com/rust-lang/rust/blob/master/src/librustc_lexer/src/lib.rs#L419 Edit: and it looks like rustc might have a special case to handle this in https://github.com/rust-lang/rust/blob/a7f28678bbf4e16893bb6a718e427504167a9494/src/libsyntax/parse/literal.rs#L424 |
Token { kind: FLOAT_NUMBER, len: TextUnit::from_usize(5)} | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing trailing newline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed (although it looks like this test might be incorrect anyway - see edit to my previous comment).
// The lexer treats e.g. `1f64` as an integer literal. See | ||
// https://github.com/rust-analyzer/rust-analyzer/issues/1592 | ||
// and the comments on the linked PR. | ||
let float_suffix_list = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the best place to handle this? It appears roughly analogous to where the special case is handled in rustc itself - see https://github.com/rust-lang/rust/blob/a7f28678bbf4e16893bb6a718e427504167a9494/src/libsyntax/parse/literal.rs#L424
Edit: or maybe we should handle this in https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ra_syntax/src/parsing/lexer.rs#L128 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is indeed the right place to do this. 1
is indeed, lexically, an integer literal
@@ -69,6 +69,21 @@ mod tests { | |||
); | |||
} | |||
|
|||
// https://github.com/rust-analyzer/rust-analyzer/issues/1592 | |||
#[test] | |||
fn add_explicit_type_infers_correct_type_for_floating_point_literal() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be more fitting to test in the type inference tests, i.e. ra_hir/hy/tests.rs
.
LGTM, but the test should be moved to |
Thanks. I've moved the test to infer, but marks don't seem to work here as the test and the code under test are in different crates (ra_hir and ra_syntax respectively). |
bors r+ Thanks! |
Build succeeded |
Fixes #1592