Skip to content

Commit

Permalink
review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Aug 13, 2019
1 parent fb2511c commit 939c1cb
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 27 deletions.
4 changes: 3 additions & 1 deletion src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,9 @@ impl<'hir> Map<'hir> {
}
}

pub fn is_const_scope(&self, hir_id: HirId) -> bool {
/// Whether the expression pointed at by `hir_id` belongs to a `const` evaluation context.
/// Used exclusively for diagnostics, to avoid suggestion function calls.
pub fn is_const_context(&self, hir_id: HirId) -> bool {
let parent_id = self.get_parent_item(hir_id);
match self.get(parent_id) {
Node::Item(&Item {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/infer/error_reporting/need_type_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let (ty_msg, suffix) = match &local_visitor.found_ty {
Some(ty) if &ty.to_string() != "_" &&
name == "_" &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized.
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) &&
!ty.is_closure() => // The suggestion doesn't make sense for closures.
{
Expand All @@ -163,7 +163,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
Some(ty) if &ty.to_string() != "_" &&
ty.to_string() != name &&
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized.
// FIXME: Remove this check after `impl_trait_in_bindings` is stabilized. #63527
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings) &&
!ty.is_closure() => // The suggestion doesn't make sense for closures.
{
Expand All @@ -185,12 +185,12 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
.map(|args| args.tuple_fields()
.map(|arg| arg.to_string())
.collect::<Vec<_>>().join(", "))
.unwrap_or_else(String::new);
.unwrap_or_default();
// This suggestion is incomplete, as the user will get further type inference
// errors due to the `_` placeholders and the introduction of `Box`, but it does
// nudge them in the right direction.
(msg, format!(
"a boxed closure type like `Box<Fn({}) -> {}>`",
"a boxed closure type like `Box<dyn Fn({}) -> {}>`",
args,
fn_sig.output().skip_binder().to_string(),
))
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
checked_ty: Ty<'tcx>,
expected_ty: Ty<'tcx>,
) -> bool {
if self.tcx.hir().is_const_scope(expr.hir_id) {
if self.tcx.hir().is_const_context(expr.hir_id) {
// Shouldn't suggest `.into()` on `const`s.
// FIXME(estebank): modify once we decide to suggest `as` casts
return false;
Expand Down
37 changes: 19 additions & 18 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3990,27 +3990,28 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected: Ty<'tcx>,
found: Ty<'tcx>,
) {
if self.tcx.hir().is_const_scope(expr.hir_id) {
if self.tcx.hir().is_const_context(expr.hir_id) {
// Do not suggest `Box::new` in const context.
return;
}
if expected.is_box() && !found.is_box() {
let boxed_found = self.tcx.mk_box(found);
if let (true, Ok(snippet)) = (
self.can_coerce(boxed_found, expected),
self.sess().source_map().span_to_snippet(expr.span),
) {
err.span_suggestion(
expr.span,
"you can store this in the heap calling `Box::new`",
format!("Box::new({})", snippet),
Applicability::MachineApplicable,
);
err.note("for more information about the distinction between the stack and the \
heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \
https://doc.rust-lang.org/rust-by-example/std/box.html and \
https://doc.rust-lang.org/std/boxed/index.html");
}
if !expected.is_box() || found.is_box() {
return;
}
let boxed_found = self.tcx.mk_box(found);
if let (true, Ok(snippet)) = (
self.can_coerce(boxed_found, expected),
self.sess().source_map().span_to_snippet(expr.span),
) {
err.span_suggestion(
expr.span,
"store this in the heap by calling `Box::new`",
format!("Box::new({})", snippet),
Applicability::MachineApplicable,
);
err.note("for more on the distinction between the stack and the \
heap, read https://doc.rust-lang.org/book/ch15-01-box.html, \
https://doc.rust-lang.org/rust-by-example/std/box.html, and \
https://doc.rust-lang.org/std/boxed/index.html");
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/inference/cannot-infer-closure.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ error[E0282]: type annotations needed for the closure
--> $DIR/cannot-infer-closure.rs:3:9
|
LL | let x = |a: (), b: ()| {
| - consider giving `x` a boxed closure type like `Box<Fn((), ()) -> std::result::Result<(), _>>`
| - consider giving `x` a boxed closure type like `Box<dyn Fn((), ()) -> std::result::Result<(), _>>`
LL | Err(a)?;
| ^^^^^^^ cannot infer type

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/suggestions/suggest-box.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ LL | | };
|
= note: expected type `std::boxed::Box<dyn std::ops::Fn() -> std::result::Result<(), ()>>`
found type `[closure@$DIR/suggest-box.rs:4:47: 7:6]`
= note: for more information about the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html and https://doc.rust-lang.org/std/boxed/index.html
help: you can store this in the heap calling `Box::new`
= note: for more on the distinction between the stack and the heap, read https://doc.rust-lang.org/book/ch15-01-box.html, https://doc.rust-lang.org/rust-by-example/std/box.html, and https://doc.rust-lang.org/std/boxed/index.html
help: store this in the heap by calling `Box::new`
|
LL | let _x: Box<dyn Fn() -> Result<(), ()>> = Box::new(|| {
LL | Err(())?;
Expand Down

0 comments on commit 939c1cb

Please sign in to comment.