Skip to content

Commit

Permalink
Rollup merge of rust-lang#63337 - estebank:break-ee0308, r=Centril
Browse files Browse the repository at this point in the history
Tweak mismatched types error

- Change expected/found for type mismatches in `break`
- Be more accurate when talking about diverging match arms
- Tweak wording of function without a return value
- Suggest calling bare functions when their return value can be coerced to the expected type
- Give more parsing errors when encountering `foo(_, _, _)`

Fix rust-lang#51767, fix rust-lang#62677, fix rust-lang#63136, cc rust-lang#37384, cc rust-lang#35241.
  • Loading branch information
Centril authored Aug 9, 2019
2 parents 4637396 + 45a5bc7 commit 3b99865
Show file tree
Hide file tree
Showing 36 changed files with 701 additions and 109 deletions.
2 changes: 1 addition & 1 deletion src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ impl<'hir> Map<'hir> {
self.definitions.def_index_to_hir_id(def_id.to_def_id().index)
}

fn def_kind(&self, hir_id: HirId) -> Option<DefKind> {
pub fn def_kind(&self, hir_id: HirId) -> Option<DefKind> {
let node = if let Some(node) = self.find(hir_id) {
node
} else {
Expand Down
38 changes: 10 additions & 28 deletions src/librustc/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -662,19 +662,22 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
}
_ => {
// `last_ty` can be `!`, `expected` will have better info when present.
let t = self.resolve_vars_if_possible(&match exp_found {
Some(ty::error::ExpectedFound { expected, .. }) => expected,
_ => last_ty,
});
let msg = "`match` arms have incompatible types";
err.span_label(cause.span, msg);
if prior_arms.len() <= 4 {
for sp in prior_arms {
err.span_label(*sp, format!(
"this is found to be of type `{}`",
self.resolve_vars_if_possible(&last_ty),
));
err.span_label( *sp, format!("this is found to be of type `{}`", t));
}
} else if let Some(sp) = prior_arms.last() {
err.span_label(*sp, format!(
"this and all prior arms are found to be of type `{}`", last_ty,
));
err.span_label(
*sp,
format!("this and all prior arms are found to be of type `{}`", t),
);
}
}
},
Expand Down Expand Up @@ -1143,27 +1146,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
(_, false, _) => {
if let Some(exp_found) = exp_found {
let (def_id, ret_ty) = match exp_found.found.sty {
ty::FnDef(def, _) => {
(Some(def), Some(self.tcx.fn_sig(def).output()))
}
_ => (None, None),
};

let exp_is_struct = match exp_found.expected.sty {
ty::Adt(def, _) => def.is_struct(),
_ => false,
};

if let (Some(def_id), Some(ret_ty)) = (def_id, ret_ty) {
if exp_is_struct && &exp_found.expected == ret_ty.skip_binder() {
let message = format!(
"did you mean `{}(/* fields */)`?",
self.tcx.def_path_str(def_id)
);
diag.span_label(span, message);
}
}
self.suggest_as_ref_where_appropriate(span, &exp_found, diag);
}

Expand Down
18 changes: 17 additions & 1 deletion src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
coerce.coerce(self, &cause, e, e_ty);
} else {
assert!(e_ty.is_unit());
coerce.coerce_forced_unit(self, &cause, &mut |_| (), true);
let ty = coerce.expected_ty();
coerce.coerce_forced_unit(self, &cause, &mut |err| {
let val = match ty.sty {
ty::Bool => "true",
ty::Char => "'a'",
ty::Int(_) | ty::Uint(_) => "42",
ty::Float(_) => "3.14159",
ty::Error | ty::Never => return,
_ => "value",
};
let msg = "give it a value of the expected type";
let label = destination.label
.map(|l| format!(" {}", l.ident))
.unwrap_or_else(String::new);
let sugg = format!("break{} {}", label, val);
err.span_suggestion(expr.span, msg, sugg, Applicability::HasPlaceholders);
}, false);
}
} else {
// If `ctxt.coerce` is `None`, we can just ignore
Expand Down
109 changes: 108 additions & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3709,7 +3709,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.consider_hint_about_removing_semicolon(blk, expected_ty, err);
}
if let Some(fn_span) = fn_span {
err.span_label(fn_span, "this function's body doesn't return");
err.span_label(
fn_span,
"implicitly returns `()` as its body has no tail or `return` \
expression",
);
}
}, false);
}
Expand Down Expand Up @@ -3819,6 +3823,101 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pointing_at_return_type
}

/// When encountering an fn-like ctor that needs to unify with a value, check whether calling
/// the ctor would successfully solve the type mismatch and if so, suggest it:
/// ```
/// fn foo(x: usize) -> usize { x }
/// let x: usize = foo; // suggest calling the `foo` function: `foo(42)`
/// ```
fn suggest_fn_call(
&self,
err: &mut DiagnosticBuilder<'tcx>,
expr: &hir::Expr,
expected: Ty<'tcx>,
found: Ty<'tcx>,
) -> bool {
match found.sty {
ty::FnDef(..) | ty::FnPtr(_) => {}
_ => return false,
}
let hir = self.tcx.hir();

let sig = found.fn_sig(self.tcx);
let sig = self
.replace_bound_vars_with_fresh_vars(expr.span, infer::FnCall, &sig)
.0;
let sig = self.normalize_associated_types_in(expr.span, &sig);
if let Ok(_) = self.try_coerce(expr, sig.output(), expected, AllowTwoPhase::No) {
let (mut sugg_call, applicability) = if sig.inputs().is_empty() {
(String::new(), Applicability::MachineApplicable)
} else {
("...".to_string(), Applicability::HasPlaceholders)
};
let mut msg = "call this function";
if let ty::FnDef(def_id, ..) = found.sty {
match hir.get_if_local(def_id) {
Some(Node::Item(hir::Item {
node: ItemKind::Fn(.., body_id),
..
})) |
Some(Node::ImplItem(hir::ImplItem {
node: hir::ImplItemKind::Method(_, body_id),
..
})) |
Some(Node::TraitItem(hir::TraitItem {
node: hir::TraitItemKind::Method(.., hir::TraitMethod::Provided(body_id)),
..
})) => {
let body = hir.body(*body_id);
sugg_call = body.arguments.iter()
.map(|arg| match &arg.pat.node {
hir::PatKind::Binding(_, _, ident, None)
if ident.name != kw::SelfLower => ident.to_string(),
_ => "_".to_string(),
}).collect::<Vec<_>>().join(", ");
}
Some(Node::Ctor(hir::VariantData::Tuple(fields, _))) => {
sugg_call = fields.iter().map(|_| "_").collect::<Vec<_>>().join(", ");
match hir.as_local_hir_id(def_id).and_then(|hir_id| hir.def_kind(hir_id)) {
Some(hir::def::DefKind::Ctor(hir::def::CtorOf::Variant, _)) => {
msg = "instantiate this tuple variant";
}
Some(hir::def::DefKind::Ctor(hir::def::CtorOf::Struct, _)) => {
msg = "instantiate this tuple struct";
}
_ => {}
}
}
Some(Node::ForeignItem(hir::ForeignItem {
node: hir::ForeignItemKind::Fn(_, idents, _),
..
})) |
Some(Node::TraitItem(hir::TraitItem {
node: hir::TraitItemKind::Method(.., hir::TraitMethod::Required(idents)),
..
})) => sugg_call = idents.iter()
.map(|ident| if ident.name != kw::SelfLower {
ident.to_string()
} else {
"_".to_string()
}).collect::<Vec<_>>()
.join(", "),
_ => {}
}
};
if let Ok(code) = self.sess().source_map().span_to_snippet(expr.span) {
err.span_suggestion(
expr.span,
&format!("use parentheses to {}", msg),
format!("{}({})", code, sugg_call),
applicability,
);
return true;
}
}
false
}

pub fn suggest_ref_or_into(
&self,
err: &mut DiagnosticBuilder<'tcx>,
Expand All @@ -3833,6 +3932,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
suggestion,
Applicability::MachineApplicable,
);
} else if let (ty::FnDef(def_id, ..), true) = (
&found.sty,
self.suggest_fn_call(err, expr, expected, found),
) {
if let Some(sp) = self.tcx.hir().span_if_local(*def_id) {
let sp = self.sess().source_map().def_span(sp);
err.span_label(sp, &format!("{} defined here", found));
}
} else if !self.check_for_cast(err, expr, found, expected) {
let is_struct_pat_shorthand_field = self.is_hir_id_from_struct_pattern_shorthand_field(
expr.hir_id,
Expand Down
70 changes: 50 additions & 20 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2052,9 +2052,23 @@ impl<'a> Parser<'a> {
while self.token != token::CloseDelim(token::Paren) {
es.push(match self.parse_expr() {
Ok(es) => es,
Err(err) => {
Err(mut err) => {
// recover from parse error in tuple list
return Ok(self.recover_seq_parse_error(token::Paren, lo, Err(err)));
match self.token.kind {
token::Ident(name, false)
if name == kw::Underscore && self.look_ahead(1, |t| {
t == &token::Comma
}) => {
// Special-case handling of `Foo<(_, _, _)>`
err.emit();
let sp = self.token.span;
self.bump();
self.mk_expr(sp, ExprKind::Err, ThinVec::new())
}
_ => return Ok(
self.recover_seq_parse_error(token::Paren, lo, Err(err)),
),
}
}
});
recovered = self.expect_one_of(
Expand Down Expand Up @@ -2456,37 +2470,35 @@ impl<'a> Parser<'a> {
}

/// Parses `a.b` or `a(13)` or `a[4]` or just `a`.
fn parse_dot_or_call_expr(&mut self,
already_parsed_attrs: Option<ThinVec<Attribute>>)
-> PResult<'a, P<Expr>> {
fn parse_dot_or_call_expr(
&mut self,
already_parsed_attrs: Option<ThinVec<Attribute>>,
) -> PResult<'a, P<Expr>> {
let attrs = self.parse_or_use_outer_attributes(already_parsed_attrs)?;

let b = self.parse_bottom_expr();
let (span, b) = self.interpolated_or_expr_span(b)?;
self.parse_dot_or_call_expr_with(b, span, attrs)
}

fn parse_dot_or_call_expr_with(&mut self,
e0: P<Expr>,
lo: Span,
mut attrs: ThinVec<Attribute>)
-> PResult<'a, P<Expr>> {
fn parse_dot_or_call_expr_with(
&mut self,
e0: P<Expr>,
lo: Span,
mut attrs: ThinVec<Attribute>,
) -> PResult<'a, P<Expr>> {
// Stitch the list of outer attributes onto the return value.
// A little bit ugly, but the best way given the current code
// structure
self.parse_dot_or_call_expr_with_(e0, lo)
.map(|expr|
self.parse_dot_or_call_expr_with_(e0, lo).map(|expr|
expr.map(|mut expr| {
attrs.extend::<Vec<_>>(expr.attrs.into());
expr.attrs = attrs;
match expr.node {
ExprKind::If(..) if !expr.attrs.is_empty() => {
// Just point to the first attribute in there...
let span = expr.attrs[0].span;

self.span_err(span,
"attributes are not yet allowed on `if` \
expressions");
self.span_err(span, "attributes are not yet allowed on `if` expressions");
}
_ => {}
}
Expand Down Expand Up @@ -2624,7 +2636,24 @@ impl<'a> Parser<'a> {
}

fn parse_paren_expr_seq(&mut self) -> PResult<'a, Vec<P<Expr>>> {
self.parse_paren_comma_seq(|p| p.parse_expr()).map(|(r, _)| r)
self.parse_paren_comma_seq(|p| {
match p.parse_expr() {
Ok(expr) => Ok(expr),
Err(mut err) => match p.token.kind {
token::Ident(name, false)
if name == kw::Underscore && p.look_ahead(1, |t| {
t == &token::Comma
}) => {
// Special-case handling of `foo(_, _, _)`
err.emit();
let sp = p.token.span;
p.bump();
Ok(p.mk_expr(sp, ExprKind::Err, ThinVec::new()))
}
_ => Err(err),
},
}
}).map(|(r, _)| r)
}

crate fn process_potential_macro_variable(&mut self) {
Expand Down Expand Up @@ -2806,9 +2835,10 @@ impl<'a> Parser<'a> {
/// This parses an expression accounting for associativity and precedence of the operators in
/// the expression.
#[inline]
fn parse_assoc_expr(&mut self,
already_parsed_attrs: Option<ThinVec<Attribute>>)
-> PResult<'a, P<Expr>> {
fn parse_assoc_expr(
&mut self,
already_parsed_attrs: Option<ThinVec<Attribute>>,
) -> PResult<'a, P<Expr>> {
self.parse_assoc_expr_with(0, already_parsed_attrs.into())
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ error[E0308]: mismatched types
LL | fn return_targets_async_block_not_fn() -> u8 {
| --------------------------------- ^^ expected u8, found ()
| |
| this function's body doesn't return
| implicitly returns `()` as its body has no tail or `return` expression
|
= note: expected type `u8`
found type `()`
Expand Down Expand Up @@ -57,7 +57,7 @@ error[E0308]: mismatched types
LL | fn rethrow_targets_async_block_not_fn() -> Result<u8, MyErr> {
| ---------------------------------- ^^^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found ()
| |
| this function's body doesn't return
| implicitly returns `()` as its body has no tail or `return` expression
|
= note: expected type `std::result::Result<u8, MyErr>`
found type `()`
Expand All @@ -68,7 +68,7 @@ error[E0308]: mismatched types
LL | fn rethrow_targets_async_block_not_async_fn() -> Result<u8, MyErr> {
| ---------------------------------------- ^^^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found ()
| |
| this function's body doesn't return
| implicitly returns `()` as its body has no tail or `return` expression
|
= note: expected type `std::result::Result<u8, MyErr>`
found type `()`
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/block-result/consider-removing-last-semi.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0308]: mismatched types
LL | fn f() -> String {
| - ^^^^^^ expected struct `std::string::String`, found ()
| |
| this function's body doesn't return
| implicitly returns `()` as its body has no tail or `return` expression
LL | 0u8;
LL | "bla".to_string();
| - help: consider removing this semicolon
Expand All @@ -18,7 +18,7 @@ error[E0308]: mismatched types
LL | fn g() -> String {
| - ^^^^^^ expected struct `std::string::String`, found ()
| |
| this function's body doesn't return
| implicitly returns `()` as its body has no tail or `return` expression
LL | "this won't work".to_string();
LL | "removeme".to_string();
| - help: consider removing this semicolon
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/block-result/issue-11714.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0308]: mismatched types
LL | fn blah() -> i32 {
| ---- ^^^ expected i32, found ()
| |
| this function's body doesn't return
| implicitly returns `()` as its body has no tail or `return` expression
...
LL | ;
| - help: consider removing this semicolon
Expand Down
Loading

0 comments on commit 3b99865

Please sign in to comment.