From 3a530baec9df99ed62a5b792da92baa6009842e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Tue, 23 Jan 2018 14:58:39 -0800 Subject: [PATCH 1/3] For E0277 on `for` loops, point at first line When E0277's span points at a `for` loop, the actual issue is in the element being iterated. Instead of pointing at the entire loop, point only at the first line (when possible) so that the span ends in the element for which E0277 was triggered. --- src/librustc/traits/error_reporting.rs | 18 ++++++++++++++++++ src/test/ui/suggestions/for-c-in-str.rs | 21 +++++++++++++++++++++ src/test/ui/suggestions/for-c-in-str.stderr | 11 +++++++++++ 3 files changed, 50 insertions(+) create mode 100644 src/test/ui/suggestions/for-c-in-str.rs create mode 100644 src/test/ui/suggestions/for-c-in-str.stderr diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index e649f1b49df76..61456b3b00379 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -551,6 +551,24 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let OnUnimplementedNote { message, label } = self.on_unimplemented_note(trait_ref, obligation); let have_alt_message = message.is_some() || label.is_some(); + let span = match self.tcx.sess.codemap().span_to_snippet(span) { + Ok(ref s) if s.starts_with("for ") => { + // On for loops, this error is caused by the element being iterated + // on, but the span points at the entire for loop. Instead of: + // + // / for c in "asdf" { + // | ... + // | } + // |_^ `&str` is not an iterator + // + // lets point at: + // + // for c in "asdf" { + // ^^^^^^^^^^^^^^^ `&str` is not an iterator + self.tcx.sess.codemap().span_until_char(span, '{') + } + _ => span, + }; let mut err = struct_span_err!( self.tcx.sess, diff --git a/src/test/ui/suggestions/for-c-in-str.rs b/src/test/ui/suggestions/for-c-in-str.rs new file mode 100644 index 0000000000000..011886e807346 --- /dev/null +++ b/src/test/ui/suggestions/for-c-in-str.rs @@ -0,0 +1,21 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// E0277 should point exclusively at line 14, not the entire for loop span + +fn main() { + for c in "asdf" { + //~^ ERROR the trait bound `&str: std::iter::Iterator` is not satisfied + //~| NOTE `&str` is not an iterator + //~| HELP the trait `std::iter::Iterator` is not implemented for `&str` + //~| NOTE required by `std::iter::IntoIterator::into_iter` + println!(""); + } +} diff --git a/src/test/ui/suggestions/for-c-in-str.stderr b/src/test/ui/suggestions/for-c-in-str.stderr new file mode 100644 index 0000000000000..e99a7d7fe55ad --- /dev/null +++ b/src/test/ui/suggestions/for-c-in-str.stderr @@ -0,0 +1,11 @@ +error[E0277]: the trait bound `&str: std::iter::Iterator` is not satisfied + --> $DIR/for-c-in-str.rs:14:5 + | +14 | for c in "asdf" { + | ^^^^^^^^^^^^^^^ `&str` is not an iterator; maybe try calling `.iter()` or a similar method + | + = help: the trait `std::iter::Iterator` is not implemented for `&str` + = note: required by `std::iter::IntoIterator::into_iter` + +error: aborting due to previous error + From f90c445637385c2526990119b3f177d809ac5459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 25 Jan 2018 14:07:45 -0800 Subject: [PATCH 2/3] Modify spans of expanded expression Modify the spans used for `for`-loop expression expansion, instead of creating a new span during error creation. --- src/librustc/hir/lowering.rs | 46 ++++++++++----------- src/librustc/traits/error_reporting.rs | 18 -------- src/test/compile-fail/issue-20605.rs | 2 +- src/test/ui/const-fn-error.stderr | 22 +++++----- src/test/ui/issue-33941.stderr | 4 +- src/test/ui/suggestions/for-c-in-str.stderr | 4 +- 6 files changed, 38 insertions(+), 58 deletions(-) diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index a87f2747a57f5..8a48dca01522b 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3021,7 +3021,7 @@ impl<'a> LoweringContext<'a> { // `match { ... }` let arms = hir_vec![pat_arm, break_arm]; - let match_expr = self.expr(e.span, + let match_expr = self.expr(sub_expr.span, hir::ExprMatch(sub_expr, arms, hir::MatchSource::WhileLetDesugar), @@ -3059,24 +3059,25 @@ impl<'a> LoweringContext<'a> { // expand let head = self.lower_expr(head); + let head_sp = head.span; let iter = self.str_to_ident("iter"); let next_ident = self.str_to_ident("__next"); - let next_pat = self.pat_ident_binding_mode(e.span, + let next_pat = self.pat_ident_binding_mode(pat.span, next_ident, hir::BindingAnnotation::Mutable); // `::std::option::Option::Some(val) => next = val` let pat_arm = { let val_ident = self.str_to_ident("val"); - let val_pat = self.pat_ident(e.span, val_ident); - let val_expr = P(self.expr_ident(e.span, val_ident, val_pat.id)); - let next_expr = P(self.expr_ident(e.span, next_ident, next_pat.id)); - let assign = P(self.expr(e.span, + let val_pat = self.pat_ident(pat.span, val_ident); + let val_expr = P(self.expr_ident(pat.span, val_ident, val_pat.id)); + let next_expr = P(self.expr_ident(pat.span, next_ident, next_pat.id)); + let assign = P(self.expr(pat.span, hir::ExprAssign(next_expr, val_expr), ThinVec::new())); - let some_pat = self.pat_some(e.span, val_pat); + let some_pat = self.pat_some(pat.span, val_pat); self.arm(hir_vec![some_pat], assign) }; @@ -3089,46 +3090,45 @@ impl<'a> LoweringContext<'a> { }; // `mut iter` - let iter_pat = self.pat_ident_binding_mode(e.span, + let iter_pat = self.pat_ident_binding_mode(head_sp, iter, hir::BindingAnnotation::Mutable); // `match ::std::iter::Iterator::next(&mut iter) { ... }` let match_expr = { - let iter = P(self.expr_ident(e.span, iter, iter_pat.id)); - let ref_mut_iter = self.expr_mut_addr_of(e.span, iter); + let iter = P(self.expr_ident(head_sp, iter, iter_pat.id)); + let ref_mut_iter = self.expr_mut_addr_of(head_sp, iter); let next_path = &["iter", "Iterator", "next"]; - let next_path = P(self.expr_std_path(e.span, next_path, ThinVec::new())); - let next_expr = P(self.expr_call(e.span, next_path, + let next_path = P(self.expr_std_path(head_sp, next_path, ThinVec::new())); + let next_expr = P(self.expr_call(head_sp, next_path, hir_vec![ref_mut_iter])); let arms = hir_vec![pat_arm, break_arm]; - P(self.expr(e.span, + P(self.expr(head_sp, hir::ExprMatch(next_expr, arms, hir::MatchSource::ForLoopDesugar), ThinVec::new())) }; - let match_stmt = respan(e.span, hir::StmtExpr(match_expr, self.next_id().node_id)); + let match_stmt = respan(head_sp, hir::StmtExpr(match_expr, self.next_id().node_id)); - let next_expr = P(self.expr_ident(e.span, next_ident, next_pat.id)); + let next_expr = P(self.expr_ident(head_sp, next_ident, next_pat.id)); // `let mut __next` - let next_let = self.stmt_let_pat(e.span, + let next_let = self.stmt_let_pat(head_sp, None, next_pat, hir::LocalSource::ForLoopDesugar); // `let = __next` let pat = self.lower_pat(pat); - let pat_let = self.stmt_let_pat(e.span, + let pat_let = self.stmt_let_pat(head_sp, Some(next_expr), pat, hir::LocalSource::ForLoopDesugar); - let body_block = self.with_loop_scope(e.id, - |this| this.lower_block(body, false)); + let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false)); let body_expr = P(self.expr_block(body_block, ThinVec::new())); - let body_stmt = respan(e.span, hir::StmtExpr(body_expr, self.next_id().node_id)); + let body_stmt = respan(body.span, hir::StmtExpr(body_expr, self.next_id().node_id)); let loop_block = P(self.block_all(e.span, hir_vec![next_let, @@ -3155,12 +3155,12 @@ impl<'a> LoweringContext<'a> { // `match ::std::iter::IntoIterator::into_iter() { ... }` let into_iter_expr = { let into_iter_path = &["iter", "IntoIterator", "into_iter"]; - let into_iter = P(self.expr_std_path(e.span, into_iter_path, + let into_iter = P(self.expr_std_path(head_sp, into_iter_path, ThinVec::new())); - P(self.expr_call(e.span, into_iter, hir_vec![head])) + P(self.expr_call(head_sp, into_iter, hir_vec![head])) }; - let match_expr = P(self.expr_match(e.span, + let match_expr = P(self.expr_match(head_sp, into_iter_expr, hir_vec![iter_arm], hir::MatchSource::ForLoopDesugar)); diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index 61456b3b00379..e649f1b49df76 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -551,24 +551,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let OnUnimplementedNote { message, label } = self.on_unimplemented_note(trait_ref, obligation); let have_alt_message = message.is_some() || label.is_some(); - let span = match self.tcx.sess.codemap().span_to_snippet(span) { - Ok(ref s) if s.starts_with("for ") => { - // On for loops, this error is caused by the element being iterated - // on, but the span points at the entire for loop. Instead of: - // - // / for c in "asdf" { - // | ... - // | } - // |_^ `&str` is not an iterator - // - // lets point at: - // - // for c in "asdf" { - // ^^^^^^^^^^^^^^^ `&str` is not an iterator - self.tcx.sess.codemap().span_until_char(span, '{') - } - _ => span, - }; let mut err = struct_span_err!( self.tcx.sess, diff --git a/src/test/compile-fail/issue-20605.rs b/src/test/compile-fail/issue-20605.rs index b7c544c78483a..5eb0e4360fc93 100644 --- a/src/test/compile-fail/issue-20605.rs +++ b/src/test/compile-fail/issue-20605.rs @@ -10,7 +10,7 @@ fn changer<'a>(mut things: Box>) { for item in *things { *item = 0 } -//~^ ERROR `std::iter::Iterator: std::marker::Sized` is not satisfied +//~^ ERROR the trait bound `std::iter::Iterator: std::marker::Sized` is not satisfied } fn main() {} diff --git a/src/test/ui/const-fn-error.stderr b/src/test/ui/const-fn-error.stderr index 0e275e78fc68c..de2a9299473fd 100644 --- a/src/test/ui/const-fn-error.stderr +++ b/src/test/ui/const-fn-error.stderr @@ -13,22 +13,20 @@ error[E0016]: blocks in constant functions are limited to items and tail express | ^ error[E0015]: calls in constant functions are limited to constant functions, struct and enum constructors - --> $DIR/const-fn-error.rs:17:5 + --> $DIR/const-fn-error.rs:17:14 | -17 | / for i in 0..x { //~ ERROR calls in constant functions -18 | | //~| ERROR constant function contains unimplemented -19 | | sum += i; -20 | | } - | |_____^ +17 | for i in 0..x { //~ ERROR calls in constant functions + | ^^^^ + | | + | in this macro invocation error[E0019]: constant function contains unimplemented expression type - --> $DIR/const-fn-error.rs:17:5 + --> $DIR/const-fn-error.rs:17:14 | -17 | / for i in 0..x { //~ ERROR calls in constant functions -18 | | //~| ERROR constant function contains unimplemented -19 | | sum += i; -20 | | } - | |_____^ +17 | for i in 0..x { //~ ERROR calls in constant functions + | ^^^^ + | | + | in this macro invocation error[E0080]: constant evaluation error --> $DIR/const-fn-error.rs:21:5 diff --git a/src/test/ui/issue-33941.stderr b/src/test/ui/issue-33941.stderr index 953e6fe77d716..78c9ce9a1b12a 100644 --- a/src/test/ui/issue-33941.stderr +++ b/src/test/ui/issue-33941.stderr @@ -8,10 +8,10 @@ error[E0271]: type mismatch resolving ` as std::iter::Iterator>::Item == &_` - --> $DIR/issue-33941.rs:14:5 + --> $DIR/issue-33941.rs:14:14 | 14 | for _ in HashMap::new().iter().cloned() {} //~ ERROR type mismatch - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected tuple, found reference + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected tuple, found reference | = note: expected type `(&_, &_)` found type `&_` diff --git a/src/test/ui/suggestions/for-c-in-str.stderr b/src/test/ui/suggestions/for-c-in-str.stderr index e99a7d7fe55ad..7a6dc9a504029 100644 --- a/src/test/ui/suggestions/for-c-in-str.stderr +++ b/src/test/ui/suggestions/for-c-in-str.stderr @@ -1,8 +1,8 @@ error[E0277]: the trait bound `&str: std::iter::Iterator` is not satisfied - --> $DIR/for-c-in-str.rs:14:5 + --> $DIR/for-c-in-str.rs:14:14 | 14 | for c in "asdf" { - | ^^^^^^^^^^^^^^^ `&str` is not an iterator; maybe try calling `.iter()` or a similar method + | ^^^^^^ `&str` is not an iterator; maybe try calling `.iter()` or a similar method | = help: the trait `std::iter::Iterator` is not implemented for `&str` = note: required by `std::iter::IntoIterator::into_iter` From 106e5c554d6b6b97aecac254a2694247e84e718e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 26 Jan 2018 11:34:12 -0800 Subject: [PATCH 3/3] Don't add "in this macro invocation" label to desugared spans --- src/librustc_errors/emitter.rs | 1 + src/test/ui/const-fn-error.stderr | 4 ---- src/test/ui/suggestions/try-on-option.stderr | 5 +---- .../ui/suggestions/try-operator-on-main.stderr | 15 +++------------ 4 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 58f851aea3817..8a4fd24a29b89 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -767,6 +767,7 @@ impl EmitterWriter { } // Check to make sure we're not in any <*macros> if !cm.span_to_filename(def_site).is_macros() && + !trace.macro_decl_name.starts_with("desugaring of ") && !trace.macro_decl_name.starts_with("#[") || always_backtrace { new_labels.push((trace.call_site, diff --git a/src/test/ui/const-fn-error.stderr b/src/test/ui/const-fn-error.stderr index de2a9299473fd..4f4f8b5ad0093 100644 --- a/src/test/ui/const-fn-error.stderr +++ b/src/test/ui/const-fn-error.stderr @@ -17,16 +17,12 @@ error[E0015]: calls in constant functions are limited to constant functions, str | 17 | for i in 0..x { //~ ERROR calls in constant functions | ^^^^ - | | - | in this macro invocation error[E0019]: constant function contains unimplemented expression type --> $DIR/const-fn-error.rs:17:14 | 17 | for i in 0..x { //~ ERROR calls in constant functions | ^^^^ - | | - | in this macro invocation error[E0080]: constant evaluation error --> $DIR/const-fn-error.rs:21:5 diff --git a/src/test/ui/suggestions/try-on-option.stderr b/src/test/ui/suggestions/try-on-option.stderr index b1be9ad3cf697..dfe950818e7e6 100644 --- a/src/test/ui/suggestions/try-on-option.stderr +++ b/src/test/ui/suggestions/try-on-option.stderr @@ -10,10 +10,7 @@ error[E0277]: the `?` operator can only be used in a function that returns `Resu --> $DIR/try-on-option.rs:23:5 | 23 | x?; //~ the `?` operator - | -- - | | - | cannot use the `?` operator in a function that returns `u32` - | in this macro invocation + | ^^ cannot use the `?` operator in a function that returns `u32` | = help: the trait `std::ops::Try` is not implemented for `u32` = note: required by `std::ops::Try::from_error` diff --git a/src/test/ui/suggestions/try-operator-on-main.stderr b/src/test/ui/suggestions/try-operator-on-main.stderr index 3b32b4a9eb7ba..e97823a3d5d5b 100644 --- a/src/test/ui/suggestions/try-operator-on-main.stderr +++ b/src/test/ui/suggestions/try-operator-on-main.stderr @@ -2,10 +2,7 @@ error[E0277]: the `?` operator can only be used in a function that returns `Resu --> $DIR/try-operator-on-main.rs:19:5 | 19 | std::fs::File::open("foo")?; //~ ERROR the `?` operator can only - | --------------------------- - | | - | cannot use the `?` operator in a function that returns `()` - | in this macro invocation + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot use the `?` operator in a function that returns `()` | = help: the trait `std::ops::Try` is not implemented for `()` = note: required by `std::ops::Try::from_error` @@ -14,10 +11,7 @@ error[E0277]: the `?` operator can only be applied to values that implement `std --> $DIR/try-operator-on-main.rs:22:5 | 22 | ()?; //~ ERROR the `?` operator can only - | --- - | | - | the `?` operator cannot be applied to type `()` - | in this macro invocation + | ^^^ the `?` operator cannot be applied to type `()` | = help: the trait `std::ops::Try` is not implemented for `()` = note: required by `std::ops::Try::into_result` @@ -38,10 +32,7 @@ error[E0277]: the `?` operator can only be applied to values that implement `std --> $DIR/try-operator-on-main.rs:32:5 | 32 | ()?; //~ ERROR the `?` operator can only - | --- - | | - | the `?` operator cannot be applied to type `()` - | in this macro invocation + | ^^^ the `?` operator cannot be applied to type `()` | = help: the trait `std::ops::Try` is not implemented for `()` = note: required by `std::ops::Try::into_result`