Provide hint when cast needs a dereference #37442

estebank · 2016-10-28T02:06:18Z

For a given code:

vec![0.0].iter().map(|s| s as i16).collect::<Vec<i16>>();

display:

error: casting `&f64` as `i16` is invalid
 --> file3.rs:2:35
  |
2 |     vec![0.0].iter().map(|s| s as i16).collect::<Vec<i16>>();
  |                              -    ^^^
  |                              |
  |                              did you mean `*s`?

instead of:

error: casting `&f64` as `i16` is invalid
 --> <anon>:2:30
  |
2 |     vec![0.0].iter().map(|s| s as i16).collect();
  |                              ^^^^^^^^
  |
  = help: cast through a raw pointer first

Fixes #37338.

rust-highfive · 2016-10-28T02:06:33Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Aatch (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

estebank · 2016-10-28T02:09:43Z

src/librustc_typeck/check/cast.rs

@@ -139,6 +140,21 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {

    fn report_cast_error(&self, fcx: &FnCtxt<'a, 'gcx, 'tcx>, e: CastError) {
        match e {
+            CastError::NeedDeref => {
+                let cast_ty = fcx.ty_to_string(self.cast_ty);
+                let mut err = fcx.type_error_struct(self.cast_span,


Pointing at the self.cast_span instead of self.span (i16 instead of s as i16 in the example) because when trying to apply the secondary label on just self.expr.span (s in the example) the actual output looks like this:

error: casting `&f64` as `i16` is invalid --> file3.rs:2:30 | 2 | vec![0.0].iter().map(|s| s as i16).collect::<Vec<i16>>(); | ^^^^^^^^ | | | casting `&f64` as `i16` is invalid // <<< this in red | try dereferencing for the cast to work // <<< this in blue

losing the fact that the secondary label's hint should only apply to the s only.

Filed #37453 for this.

nagisa · 2016-10-28T14:41:04Z

src/librustc_typeck/check/cast.rs

+                                       self.expr_ty);
+                err.span_label(self.cast_span,
+                               &format!("casting `{}` as `{}` is invalid", self.expr_ty, cast_ty));
+                err.span_label(self.expr.span,


This shouldn’t be a span_label, but rather span_help instead.

cc @jonathandturner

If we show the deref, it should be span_help.

Another way would be to change the label to, for example, "did you mean *{}?"

nagisa · 2016-10-28T14:42:25Z

src/librustc_typeck/check/cast.rs

-                                       CastError::NeedViaUsize => "a usize",
-                                       _ => bug!(),
-                                   }))
+                    .span_label(self.span,


I disagree about label here as well.

nagisa · 2016-10-28T14:46:37Z

src/librustc_typeck/check/cast.rs

+                        }
+                    }
+                    _ => Err(CastError::NeedViaPtr),
+                }


struct A; &A as A;

stays without a note/help. I’m fine with it personally, but that’s something that could use such a help message.

estebank · 2016-11-09T20:20:03Z

r? @jonathandturner

sophiajt · 2016-11-12T13:16:09Z

@estebank - I like it! Can we do one small tweak while you're at it?

error: casting `&f64` as `i16` is invalid
 --> foo.rs:2:35
  |
2 |     vec![0.0].iter().map(|s| s as i16).collect::<Vec<i16>>();
  |                              -    ^^^ cannot cast `&f64` as `i64`
  |                              |
  |                              did you mean `*s`?

estebank · 2016-11-14T16:57:27Z

@jonathandturner done!

sophiajt · 2016-11-14T17:01:16Z

Great!

@bors r+ rollup

bors · 2016-11-14T17:01:33Z

📌 Commit f291bbe has been approved by jonathandturner

bors · 2016-11-17T05:03:32Z

☔ The latest upstream changes (presumably #37375) made this pull request unmergeable. Please resolve the merge conflicts.

Rollup of 8 pull requests - Successful merges: #37752, #37757, #37759, #37766, #37772, #37799, #37806, #37821 - Failed merges: #37442

estebank · 2016-11-20T01:18:47Z

@jonathandturner fixed merge conflict.

sophiajt · 2016-11-22T17:30:27Z

@bors r+ rollup

bors · 2016-11-22T17:30:28Z

📌 Commit ec24442 has been approved by jonathandturner

…ndturner Provide hint when cast needs a dereference For a given code: ``` rust vec![0.0].iter().map(|s| s as i16).collect::<Vec<i16>>(); ``` display: ``` nocode error: casting `&f64` as `i16` is invalid --> file3.rs:2:35 | 2 | vec![0.0].iter().map(|s| s as i16).collect::<Vec<i16>>(); | - ^^^ | | | did you mean `*s`? ``` instead of: ``` nocode error: casting `&f64` as `i16` is invalid --> <anon>:2:30 | 2 | vec![0.0].iter().map(|s| s as i16).collect(); | ^^^^^^^^ | = help: cast through a raw pointer first ``` Fixes rust-lang#37338.

Rollup of 7 pull requests - Successful merges: #37442, #37760, #37836, #37851, #37859, #37913, #37925 - Failed merges:

rust-highfive assigned Aatch Oct 28, 2016

estebank commented Oct 28, 2016

View reviewed changes

estebank mentioned this pull request Oct 28, 2016

Improve reference cast help message #37375

Merged

estebank force-pushed the cast-deref-hint branch 2 times, most recently from 42f99b7 to 3790a03 Compare October 28, 2016 07:37

nagisa reviewed Oct 28, 2016

View reviewed changes

estebank mentioned this pull request Oct 28, 2016

Be explicit when spans are overlapping in error message #37453

Closed

estebank force-pushed the cast-deref-hint branch 4 times, most recently from f85bf8f to 5a5df0a Compare October 29, 2016 00:38

rust-highfive assigned sophiajt and unassigned Aatch Nov 9, 2016

estebank force-pushed the cast-deref-hint branch from a843721 to f291bbe Compare November 13, 2016 03:12

This was referenced Nov 17, 2016

Rollup of 8 pull requests #37830

Closed

Rollup of 8 pull requests #37837

Merged

bors added a commit that referenced this pull request Nov 17, 2016

Auto merge of #37837 - GuillaumeGomez:rollup, r=GuillaumeGomez

89386d6

Rollup of 8 pull requests - Successful merges: #37752, #37757, #37759, #37766, #37772, #37799, #37806, #37821 - Failed merges: #37442

estebank force-pushed the cast-deref-hint branch from f291bbe to ec24442 Compare November 20, 2016 01:18

GuillaumeGomez mentioned this pull request Nov 22, 2016

Rollup of 7 pull requests #37937

Merged

bors added a commit that referenced this pull request Nov 23, 2016

Auto merge of #37937 - GuillaumeGomez:rollup, r=GuillaumeGomez

127a83d

Rollup of 7 pull requests - Successful merges: #37442, #37760, #37836, #37851, #37859, #37913, #37925 - Failed merges:

bors merged commit ec24442 into rust-lang:master Nov 23, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide hint when cast needs a dereference #37442

Provide hint when cast needs a dereference #37442

estebank commented Oct 28, 2016 •

edited

Loading

rust-highfive commented Oct 28, 2016

estebank Oct 28, 2016

estebank Oct 28, 2016

nagisa Oct 28, 2016

nagisa Oct 28, 2016

sophiajt Oct 28, 2016

nagisa Oct 28, 2016

nagisa Oct 28, 2016

estebank commented Nov 9, 2016

sophiajt commented Nov 12, 2016

estebank commented Nov 14, 2016

sophiajt commented Nov 14, 2016

bors commented Nov 14, 2016

bors commented Nov 17, 2016

estebank commented Nov 20, 2016

sophiajt commented Nov 22, 2016

bors commented Nov 22, 2016

Provide hint when cast needs a dereference #37442

Provide hint when cast needs a dereference #37442

Conversation

estebank commented Oct 28, 2016 • edited Loading

rust-highfive commented Oct 28, 2016

estebank Oct 28, 2016

Choose a reason for hiding this comment

estebank Oct 28, 2016

Choose a reason for hiding this comment

nagisa Oct 28, 2016

Choose a reason for hiding this comment

nagisa Oct 28, 2016

Choose a reason for hiding this comment

sophiajt Oct 28, 2016

Choose a reason for hiding this comment

nagisa Oct 28, 2016

Choose a reason for hiding this comment

nagisa Oct 28, 2016

Choose a reason for hiding this comment

estebank commented Nov 9, 2016

sophiajt commented Nov 12, 2016

estebank commented Nov 14, 2016

sophiajt commented Nov 14, 2016

bors commented Nov 14, 2016

bors commented Nov 17, 2016

estebank commented Nov 20, 2016

sophiajt commented Nov 22, 2016

bors commented Nov 22, 2016

estebank commented Oct 28, 2016 •

edited

Loading