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

When cast needs a dereference point at full cast #38245

Merged
merged 1 commit into from
Dec 21, 2016

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Dec 8, 2016

After the fix of #37453 in PR #37369, instead of pointing at only the cast type, point at the full cast span when a cast needs a dereference, as well as assign the error label to the correct span for proper coloring:

error span pointing at the entire cast

instead of

error span pointing at the type of the cast

Move compile-fail test to ui test.

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@nikomatsakis
Copy link
Contributor

@emilio

Hmm I'm not sure I see this as an improvement...I think I prefer the new span, but I'd prefer the "did you mean *s?" bit to be moved into a Help, so that it doesn't overlap. What do you think @jonathandturner ?

@estebank estebank force-pushed the cast-deref-hint-2 branch 2 times, most recently from 81bd4cb to 096205b Compare December 14, 2016 21:44
After the fix of rust-lang#37453 in PR rust-lang#37369, instead of pointing at only the
cast type, point at the full cast span when a cast needs a dereference:

```
error: casting `&{float}` as `f32` is invalid
  --> ../../../src/test/ui/mismatched_types/cast-rfc0401.rs:81:30
   |
81 |     vec![0.0].iter().map(|s| s as f32).collect::<Vec<f32>>();
   |                              ^^^^^^^^ cannot cast `&{float}` as `f32`
   |
help: did you mean `*s`?
  --> ../../../src/test/ui/mismatched_types/cast-rfc0401.rs:81:30
   |
81 |     vec![0.0].iter().map(|s| s as f32).collect::<Vec<f32>>();
   |                              ^
```

instead of

```
error: casting `&{float}` as `f32` is invalid
  --> ../../../src/test/ui/mismatched_types/cast-rfc0401.rs:81:35
   |
81 |     vec![0.0].iter().map(|s| s as f32).collect::<Vec<f32>>();
   |                              -    ^^^
   |                              |
   |                              |
   |                              did you mean `*s`?
   |                              cannot cast `&{float}` as `f32`
```
@estebank
Copy link
Contributor Author

estebank commented Dec 15, 2016

@nikomatsakis I have separated the suggestion to a help as requested:

error: casting `&{float}` as `f32` is invalid
  --> $DIR/cast-rfc0401.rs:81:30
   |
81 |     vec![0.0].iter().map(|s| s as f32).collect::<Vec<f32>>();
   |                              ^^^^^^^^ cannot cast `&{float}` as `f32`
   |
help: did you mean `*s`?
  --> $DIR/cast-rfc0401.rs:81:30
   |
81 |     vec![0.0].iter().map(|s| s as f32).collect::<Vec<f32>>();
   |                              ^

@nikomatsakis
Copy link
Contributor

@estebank looks good!

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Dec 19, 2016

📌 Commit 868fb03 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 20, 2016

⌛ Testing commit 868fb03 with merge 13c4e28...

bors added a commit that referenced this pull request Dec 20, 2016
When cast needs a dereference point at full cast

After the fix of #37453 in PR #37369, instead of pointing at only the cast type, point at the full cast span when a cast needs a dereference, as well as assign the error label to the correct span for proper coloring:

<img width="471" alt="error span pointing at the entire cast" src="https://cloud.githubusercontent.com/assets/1606434/21024245/8797fc2e-bd38-11e6-82c1-66c281c656c1.png">

instead of

<img width="471" alt="error span pointing at the type of the cast" src="https://cloud.githubusercontent.com/assets/1606434/21023777/d4814aa6-bd36-11e6-9fc3-b2a0ea5ee15d.png">

Move `compile-fail` test to `ui` test.
@eddyb
Copy link
Member

eddyb commented Dec 20, 2016

@rust-lang/tools tests failed but homu didn't pick it up.

@bors r- force

@eddyb
Copy link
Member

eddyb commented Dec 20, 2016

Hmmmmmm let's try this instead (sorry, @estebank, trying to get the bot unstuck).

@eddyb eddyb closed this Dec 20, 2016
@eddyb
Copy link
Member

eddyb commented Dec 20, 2016

Well that didn't help anything 😞.

@eddyb eddyb reopened this Dec 20, 2016
@brson
Copy link
Contributor

brson commented Dec 20, 2016

@bors retry

@alexcrichton
Copy link
Member

@bors: r=nikomatsakis

@bors
Copy link
Contributor

bors commented Dec 20, 2016

📌 Commit 868fb03 has been approved by nikomatsakis

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Dec 20, 2016
…atsakis

When cast needs a dereference point at full cast

After the fix of rust-lang#37453 in PR rust-lang#37369, instead of pointing at only the cast type, point at the full cast span when a cast needs a dereference, as well as assign the error label to the correct span for proper coloring:

<img width="471" alt="error span pointing at the entire cast" src="https://cloud.githubusercontent.com/assets/1606434/21024245/8797fc2e-bd38-11e6-82c1-66c281c656c1.png">

instead of

<img width="471" alt="error span pointing at the type of the cast" src="https://cloud.githubusercontent.com/assets/1606434/21023777/d4814aa6-bd36-11e6-9fc3-b2a0ea5ee15d.png">

Move `compile-fail` test to `ui` test.
bors added a commit that referenced this pull request Dec 20, 2016
@bors bors merged commit 868fb03 into rust-lang:master Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants