Fix invalid float literal suggestions when recovering an integer #105650

cassaundra · 2022-12-13T07:06:11Z

Only suggest adding a zero to integers with a preceding dot when the change will result in a valid floating point literal.

For example, .0x0 should not be turned into 0.0x0.

r? nnethercote

compiler/rustc_parse/src/parser/expr.rs

cassaundra · 2022-12-13T07:09:12Z

src/test/ui/suggestions/recover-invalid-float-invalid.rs

@@ -0,0 +1,21 @@
+fn main() {}
+
+fn a() {


Not sure if there's a better way to test errors like this other than putting them in separate functions/items.

They can all go in a single function.

Unfortunately it seems having two of these errors in a single function means only the first will be tested, which is why I had them in separate functions. Just curious if there's a more idiomatic way to do this, but I'm not seeing anything. Not blocking.

How are you writing the test? There are lots of examples in the test suite with more than one error per function, e.g. src/test/ui/attrs-resolution-errors.rs. Also see https://rustc-dev-guide.rust-lang.org/tests/ui.html for docs about writing UI tests.

How are you writing the test?

Something like the following fails prematurely after the first error, hence the separate functions:

fn main() { _ = .3u32; //~^ ERROR expected expression, found `.` _ = .0b0; //~^ ERROR expected expression, found `.` _ = .0o07; //~^ ERROR expected expression, found `.` _ = .0x0ABC; //~^ ERROR expected expression, found `.` }

Oh, I see, it's nothing to do with the test harness, it's the parser itself no longer able to recover. Hmm.

@compiler-errors: This PR is about some error suggestions relating to invalid literals. E.g. if you have .3u32 the compiler current suggests:

error: float literals must have an integer part --> c.rs:2:9 | 2 | _ = .3u32; | ^^^^^ help: must have an integer part: `0.3u32`

But that suggestion is bogus, because 0.3u32 isn't a valid literal:

error: invalid suffix `u32` for float literal --> c.rs:2:9 | 2 | _ = 0.3u32; | ^^^^^^ invalid suffix `u32` | = help: valid suffixes are `f32` and `f64`

So I suggested that @cassaundra fix this, and this PR avoids the suggestion if the suffix isn't f32/f64, changing the error to:

error: expected expression, found `.` --> c.rs:2:9 | 2 | _ = .3u32; | ^ expected expression

This also prevents error recovery from happening, so any subsequent parse errors in the function aren't picked up.

Do you think this change is good enough as is? .3u32 is weird enough that I'm not sure if trying to recover makes sense, but I'm no expert on error messages.

@nnethercote yes, failing the parse and consuming the rest of the block is unfortunate but acceptable. We can file a ticket to follow up on this and attempt to recover by discarding only until the ; (which I thought we did already in statements, but there might be some reason it doesn't trigger here). Splitting the test in one function per unparseable case should be enough to get around this issue in the tests.

nnethercote

Looking good, just some minor suggestions.

nnethercote · 2022-12-13T11:12:15Z

src/test/ui/suggestions/recover-invalid-float-invalid.rs

@@ -0,0 +1,21 @@
+fn main() {}
+
+fn a() {


They can all go in a single function.

src/test/ui/suggestions/recover-invalid-float-invalid.rs

compiler/rustc_parse/src/parser/expr.rs

cassaundra · 2022-12-16T02:16:04Z

Discussion aside, requested comments have been added.

pnkfelix · 2023-01-19T15:17:38Z

@bors r+

(I think the test is fine. If we end up improving parser recovery to enable such tests to be more concise, that would be great, but I don't think it makes sense for that to block landing this PR.)

bors · 2023-01-19T15:17:41Z

📌 Commit 7497f7c has been approved by pnkfelix

It is now in the queue for this repository.

…n, r=pnkfelix Fix invalid float literal suggestions when recovering an integer Only suggest adding a zero to integers with a preceding dot when the change will result in a valid floating point literal. For example, `.0x0` should not be turned into `0.0x0`. r? nnethercote

matthiaskrgr · 2023-01-20T06:24:46Z

@bors r-
tests moved from src/test to tests

nnethercote · 2023-01-30T05:46:57Z

@cassaundra This just needs to be updated for the recent src/test/*-to-tests/* renaming. Then it will be ready to merge.

Only suggest adding a zero to integers with a preceding dot when the change will result in a valid floating point literal. For example, `.0x0` should not be turned into `0.0x0`.

cassaundra · 2023-01-30T21:44:12Z

Ah apologies, I missed that. Should be fixed now.

cassaundra · 2023-01-30T22:07:33Z

@rustbot ready

estebank · 2023-01-30T22:25:36Z

@bors r=pnkfelix

bors · 2023-01-30T22:25:38Z

📌 Commit 80fcd7c has been approved by pnkfelix

It is now in the queue for this repository.

bors · 2023-01-31T00:03:11Z

⌛ Testing commit 80fcd7c with merge 487e83b...

bors · 2023-01-31T02:46:05Z

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing 487e83b to master...

rust-timer · 2023-01-31T04:05:17Z

Finished benchmarking commit (487e83b): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.4%	[-5.4%, -5.4%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.8%	[-3.8%, -3.8%]	1
Improvements ✅ (secondary)	-2.6%	[-4.3%, -1.0%]	2
All ❌✅ (primary)	-3.8%	[-3.8%, -3.8%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.4%	[-3.4%, -3.4%]	1
All ❌✅ (primary)	-	-	0

rustbot assigned nnethercote Dec 13, 2022

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 13, 2022

cassaundra commented Dec 13, 2022

View reviewed changes

nnethercote reviewed Dec 13, 2022

View reviewed changes

cassaundra force-pushed the float-literal-suggestion branch from d4a886e to 7497f7c Compare December 16, 2022 02:15

pnkfelix added the A-diagnostics Area: Messages for errors, warnings, and lints label Jan 19, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 19, 2023

matthiaskrgr mentioned this pull request Jan 20, 2023

Rollup of 7 pull requests #107104

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 20, 2023

Fix invalid float literal suggestions when recovering an integer

80fcd7c

Only suggest adding a zero to integers with a preceding dot when the change will result in a valid floating point literal. For example, `.0x0` should not be turned into `0.0x0`.

cassaundra force-pushed the float-literal-suggestion branch from 7497f7c to 80fcd7c Compare January 30, 2023 21:42

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 30, 2023

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 31, 2023

bors merged commit 487e83b into rust-lang:master Jan 31, 2023

rustbot added this to the 1.69.0 milestone Jan 31, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix invalid float literal suggestions when recovering an integer #105650

Fix invalid float literal suggestions when recovering an integer #105650

cassaundra commented Dec 13, 2022

cassaundra Dec 13, 2022

nnethercote Dec 13, 2022

cassaundra Dec 14, 2022

nnethercote Dec 14, 2022

cassaundra Dec 14, 2022 •

edited

Loading

nnethercote Dec 14, 2022

estebank Jan 19, 2023

nnethercote left a comment

nnethercote Dec 13, 2022

cassaundra commented Dec 16, 2022

pnkfelix commented Jan 19, 2023

bors commented Jan 19, 2023

matthiaskrgr commented Jan 20, 2023

nnethercote commented Jan 30, 2023

cassaundra commented Jan 30, 2023

cassaundra commented Jan 30, 2023

estebank commented Jan 30, 2023

bors commented Jan 30, 2023

bors commented Jan 31, 2023

bors commented Jan 31, 2023

rust-timer commented Jan 31, 2023

Fix invalid float literal suggestions when recovering an integer #105650

Fix invalid float literal suggestions when recovering an integer #105650

Conversation

cassaundra commented Dec 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cassaundra Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nnethercote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cassaundra commented Dec 16, 2022

pnkfelix commented Jan 19, 2023

bors commented Jan 19, 2023

matthiaskrgr commented Jan 20, 2023

nnethercote commented Jan 30, 2023

cassaundra commented Jan 30, 2023

cassaundra commented Jan 30, 2023

estebank commented Jan 30, 2023

bors commented Jan 30, 2023

bors commented Jan 31, 2023

bors commented Jan 31, 2023

rust-timer commented Jan 31, 2023

Overall result: ✅ improvements - no action needed

cassaundra Dec 14, 2022 •

edited

Loading