Skip to content

Fix suggestion on DerefMut #71389

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

Open
pickfire opened this issue Apr 21, 2020 · 2 comments
Open

Fix suggestion on DerefMut #71389

pickfire opened this issue Apr 21, 2020 · 2 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pickfire
Copy link
Contributor

pickfire commented Apr 21, 2020

I tried this code:

fn t(n: &mut i32) {
    n = "42".parse().unwrap();
}

I expected to see this happen: Suggestion of *n = ..., the correct code would be

fn t(n: &mut i32) {
    *n = "42".parse().unwrap();
}

Instead, this happened: It did not look at mutable deref value and directly show the suggestion

error[E0277]: the trait bound `&mut i32: std::str::FromStr` is not satisfied
 --> src/lib.rs:2:14
  |
2 |     n = "42".parse().unwrap();
  |              ^^^^^ the trait `std::str::FromStr` is not implemented for `&mut i32`
  |
  = help: the following implementations were found:
            <i32 as std::str::FromStr>

Not sure why but I miss * quite often. Link to playground https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=011e594936166764d011c581d7db107e

Meta

rustc --version --verbose:

1.42.0
@pickfire pickfire added the C-bug Category: This is a bug. label Apr 21, 2020
@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed C-bug Category: This is a bug. labels Apr 21, 2020
@pickfire
Copy link
Contributor Author

@estebank Do you have any mentor instructions for this? I did asked in discord once but you said go ahead. If not I would need to grep myself. :D

@estebank
Copy link
Contributor

estebank commented Oct 1, 2020

@pickfire

$ RUST_BACKTRACE=1 rustc +devel fil7.rs -Ztreat-err-as-bug
error[E0277]: the trait bound `&mut i32: FromStr` is not satisfied
 --> fil7.rs:2:14
  |
2 |     n = "42".parse().unwrap();
  |              ^^^^^ the trait `FromStr` is not implemented for `&mut i32`
  |
  = help: the following implementations were found:
            <i32 as FromStr>

thread 'rustc' panicked at 'aborting due to `-Z treat-err-as-bug=1`', compiler/rustc_errors/src/lib.rs:977:27
stack backtrace:
   0: std::panicking::begin_panic
   1: rustc_errors::HandlerInner::emit_diagnostic
   2: rustc_errors::Handler::emit_diagnostic
   3: rustc_errors::diagnostic_builder::DiagnosticBuilder::emit
   4: <rustc_infer::infer::InferCtxt as rustc_trait_selection::traits::error_reporting::InferCtxtExt>::report_selection_error
   5: <rustc_infer::infer::InferCtxt as rustc_trait_selection::traits::error_reporting::InferCtxtPrivExt>::report_fulfillment_error
   6: <rustc_infer::infer::InferCtxt as rustc_trait_selection::traits::error_reporting::InferCtxtExt>::report_fulfillment_errors
   7: rustc_typeck::check::fn_ctxt::FnCtxt::select_obligations_where_possible
   8: rustc_infer::infer::InferCtxtBuilder::enter
   9: rustc_typeck::check::inherited::InheritedBuilder::enter
  10: rustc_typeck::check::typeck_with_fallback
  11: rustc_typeck::check::typeck
  12: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::typeck>::compute
  13: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  14: rustc_data_structures::stack::ensure_sufficient_stack
  15: rustc_query_system::query::plumbing::get_query_impl
  16: rustc_query_system::query::plumbing::ensure_query_impl
  17: rustc_middle::ty::<impl rustc_middle::ty::context::TyCtxt>::par_body_owners
  18: rustc_typeck::check::typeck_item_bodies
  19: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::typeck_item_bodies>::compute
  20: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  21: rustc_data_structures::stack::ensure_sufficient_stack
  22: rustc_query_system::query::plumbing::get_query_impl
  23: rustc_session::utils::<impl rustc_session::session::Session>::time
  24: rustc_typeck::check_crate
  25: rustc_interface::passes::analysis
  26: rustc_middle::ty::query::<impl rustc_query_system::query::config::QueryAccessors<rustc_middle::ty::context::TyCtxt> for rustc_middle::ty::query::queries::analysis>::compute
  27: rustc_query_system::dep_graph::graph::DepGraph<K>::with_task_impl
  28: rustc_query_system::dep_graph::graph::DepGraph<K>::with_eval_always_task
  29: rustc_data_structures::stack::ensure_sufficient_stack
  30: rustc_query_system::query::plumbing::get_query_impl
  31: rustc_interface::passes::QueryContext::enter
  32: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
  33: rustc_span::with_source_map
  34: rustc_interface::interface::create_compiler_and_run
  35: scoped_tls::ScopedKey<T>::set
  36: rustc_span::with_session_globals
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.48.0-dev running on x86_64-apple-darwin

note: compiler flags: -Z treat-err-as-bug

query stack during panic:
#0 [typeck] type-checking `t`
#1 [typeck_item_bodies] type-checking all item bodies
#2 [analysis] running analysis passes on this crate
end of query stack

Searching for report_selection_error:

Looking on its body, you can see how we suggest borrowing (in this case we would want to check the opposite):

/// When after several dereferencing, the reference satisfies the trait
/// binding. This function provides dereference suggestion for this
/// specific situation.
fn suggest_dereferences(
&self,
obligation: &PredicateObligation<'tcx>,
err: &mut DiagnosticBuilder<'tcx>,
trait_ref: &ty::PolyTraitRef<'tcx>,
points_at_arg: bool,
) {
// It only make sense when suggesting dereferences for arguments
if !points_at_arg {
return;
}
let param_env = obligation.param_env;
let body_id = obligation.cause.body_id;
let span = obligation.cause.span;
let real_trait_ref = match &obligation.cause.code {
ObligationCauseCode::ImplDerivedObligation(cause)
| ObligationCauseCode::DerivedObligation(cause)
| ObligationCauseCode::BuiltinDerivedObligation(cause) => &cause.parent_trait_ref,
_ => trait_ref,
};
let real_ty = match real_trait_ref.self_ty().no_bound_vars() {
Some(ty) => ty,
None => return,
};
if let ty::Ref(region, base_ty, mutbl) = *real_ty.kind() {
let mut autoderef = Autoderef::new(self, param_env, body_id, span, base_ty, span);
if let Some(steps) = autoderef.find_map(|(ty, steps)| {
// Re-add the `&`
let ty = self.tcx.mk_ref(region, TypeAndMut { ty, mutbl });
let obligation =
self.mk_trait_obligation_with_new_self_ty(param_env, real_trait_ref, ty);
Some(steps).filter(|_| self.predicate_may_hold(&obligation))
}) {
if steps > 0 {
if let Ok(src) = self.tcx.sess.source_map().span_to_snippet(span) {
// Don't care about `&mut` because `DerefMut` is used less
// often and user will not expect autoderef happens.
if src.starts_with('&') && !src.starts_with("&mut ") {
let derefs = "*".repeat(steps);
err.span_suggestion(
span,
"consider adding dereference here",
format!("&{}{}", derefs, &src[1..]),
Applicability::MachineApplicable,
);
}
}
}
}
}
}

I don't know if we will be able to provide a structured suggestion, because to do so we would need to have some way of recovering the enclosing expression to identify it is an assignment, and from knowing the type we could suggest *n =, but don't think we have that info available here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants