Skip to content

Commit

Permalink
When annotations needed, look at impls for more accurate suggestions
Browse files Browse the repository at this point in the history 
When encountering an expression that needs type annotations, if we have the trait `DefId` we look for all the `impl`s that could be satisfied by the expression we have (without looking at additional obligations) and suggest the fully qualified to specific impls. For left over type parameters, we replace them with `_`.

```
error[E0283]: type annotations needed
  --> $DIR/E0283.rs:35:24
   |
LL |     let bar = foo_impl.into() * 1u32;
   |                        ^^^^
   |
note: multiple `impl`s satisfying `Impl: Into<_>` found
  --> $DIR/E0283.rs:17:1
   |
LL | impl Into<u32> for Impl {
   | ^^^^^^^^^^^^^^^^^^^^^^^
   = note: and another `impl` found in the `core` crate:
           - impl<T, U> Into<U> for T
             where U: From<T>;
help: try using a fully qualified path to specify the expected types
   |
LL |     let bar = <_ as Into<_>>::into(foo_impl) * 1u32;
   |               +++++++++++++++++++++        ~
help: try using a fully qualified path to specify the expected types
   |
LL |     let bar = <Impl as Into<u32>>::into(foo_impl) * 1u32;
   |               ++++++++++++++++++++++++++        ~
```

This approach does not account for blanket-impls, so we can end up with suggestions like `<_ as Into<_>>::into(foo)`. It'd be nice to have a more complete mechanism that does account for all obligations when resolving methods.

Do not suggest `path::to<impl Trait for Type>::method`

In the pretty-printer, we have a weird way to display fully-qualified for non-local impls, where we show a regular path, but the section corresponding to the `<Type as Trait>` we use non-syntax for it like `path::to<impl Trait for Type>`. It should be `<Type for path::to::Trait>`, but this is only better when we are printing code to be suggested, not to find where the `impl` actually is, so we add a new flag to the printer for this.

Special case `Into` suggestion to look for `From` `impl`s

When we encounter a blanket `<Ty as Into<Other>` `impl`, look at the `From` `impl`s so that we can suggest the appropriate `Other`:

```
error[E0284]: type annotations needed
  --> $DIR/issue-70082.rs:7:33
   |
LL |     let y: f64 = 0.01f64 * 1i16.into();
   |                          -      ^^^^
   |                          |
   |                          type must be known at this point
   |
   = note: cannot satisfy `<f64 as Mul<_>>::Output == f64`
help: try using a fully qualified path to specify the expected types
   |
LL |     let y: f64 = 0.01f64 * <i16 as Into<i32>>::into(1i16);
   |                            +++++++++++++++++++++++++    ~
help: try using a fully qualified path to specify the expected types
   |
LL |     let y: f64 = 0.01f64 * <i16 as Into<i64>>::into(1i16);
   |                            +++++++++++++++++++++++++    ~
help: try using a fully qualified path to specify the expected types
   |
LL |     let y: f64 = 0.01f64 * <i16 as Into<i128>>::into(1i16);
   |                            ++++++++++++++++++++++++++    ~
help: try using a fully qualified path to specify the expected types
   |
LL |     let y: f64 = 0.01f64 * <i16 as Into<isize>>::into(1i16);
   |                            +++++++++++++++++++++++++++    ~
help: try using a fully qualified path to specify the expected types
   |
LL |     let y: f64 = 0.01f64 * <i16 as Into<f32>>::into(1i16);
   |                            +++++++++++++++++++++++++    ~
help: try using a fully qualified path to specify the expected types
   |
LL |     let y: f64 = 0.01f64 * <i16 as Into<f64>>::into(1i16);
   |                            +++++++++++++++++++++++++    ~
help: try using a fully qualified path to specify the expected types
   |
LL |     let y: f64 = 0.01f64 * <i16 as Into<AtomicI16>>::into(1i16);
   |                            +++++++++++++++++++++++++++++++    ~
```

Suggest an appropriate type for a binding of method chain

Do the same we do with fully-qualified type suggestions to the suggestion to specify a binding type:

```
error[E0282]: type annotations needed
  --> $DIR/slice-pattern-refutable.rs:14:9
   |
LL |     let [a, b, c] = Zeroes.into() else {
   |         ^^^^^^^^^
   |
help: consider giving this pattern a type
   |
LL |     let [a, b, c]: _ = Zeroes.into() else {
   |                  +++
help: consider giving this pattern a type
   |
LL |     let [a, b, c]: [usize; 3] = Zeroes.into() else {
   |                  ++++++++++++
```

review comments

 - Pass `ParamEnv` through
 - Remove now-unnecessary `Formatter` mode
 - Rework the way we pick up the bounds

Add naïve mechanism to filter `Into` suggestions involving math ops

```
error[E0284]: type annotations needed
  --> $DIR/issue-70082.rs:7:33
   |
LL |     let y: f64 = 0.01f64 * 1i16.into();
   |                          -      ^^^^
   |                          |
   |                          type must be known at this point
   |
   = note: cannot satisfy `<f64 as Mul<_>>::Output == f64`
help: try using a fully qualified path to specify the expected types
   |
LL |     let y: f64 = 0.01f64 * <i16 as Into<f64>>::into(1i16);
   |                            +++++++++++++++++++++++++    ~
```

Note that we only suggest `Into<f64>`, and not `Into<i32>`, `Into<i64>`, `Into<i128>`, `Into<isize>`, `Into<f32>` or `Into<AtomicI16>`.

Replace `_` with `/* Type */` in let binding type suggestion

Rework the `predicate` "trafficking" to be more targetted

Rename `predicate` to `originating_projection`. Pass in only the `ProjectionPredicate` instead of the `Predicate` to avoid needing to destructure as much.
  • Loading branch information
@estebank
estebank committed Aug 20, 2024
1 parent 4d5b3b1 commit adc3ea6
Show file tree
Hide file tree
Showing 32 changed files with 652 additions and 112 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1548,6 +1548,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty.into(),
TypeAnnotationNeeded::E0282,
true,
self.param_env,
None,
)
.emit()
});
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/src/method/probe.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty.into(),
TypeAnnotationNeeded::E0282,
!raw_ptr_call,
self.param_env,
None,
);
if raw_ptr_call {
err.span_label(span, "cannot call a method on a raw pointer with an unknown pointee type");
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/src/writeback.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,8 @@ impl<'cx, 'tcx> Resolver<'cx, 'tcx> {
p.into(),
TypeAnnotationNeeded::E0282,
false,
self.fcx.param_env,
None,
)
.emit()
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ symbols! {
bitxor,
bitxor_assign,
black_box,
blanket_into_impl,
block,
bool,
borrowck_graphviz_format,
Expand Down
233 changes: 213 additions & 20 deletions compiler/rustc_trait_selection/src/error_reporting/infer/need_type_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::errors::{
AmbiguousImpl, AmbiguousReturn, AnnotationRequired, InferenceBadError,
SourceKindMultiSuggestion, SourceKindSubdiag,
};
use crate::infer::InferCtxt;
use crate::infer::{InferCtxt, InferCtxtExt};

pub enum TypeAnnotationNeeded {
/// ```compile_fail,E0282
Expand Down Expand Up @@ -74,6 +74,14 @@ pub enum UnderspecifiedArgKind {
Const { is_parameter: bool },
}

enum InferenceSuggestionFormat {
/// The inference suggestion will the provided as the explicit type of a binding.
BindingType,
/// The inference suggestion will the provided in the same expression where the error occurred,
/// expanding method calls into fully-qualified paths specifying the self-type and trait.
FullyQualifiedMethodCall,
}

impl InferenceDiagnosticsData {
fn can_add_more_info(&self) -> bool {
!(self.name == "_" && matches!(self.kind, UnderspecifiedArgKind::Type { .. }))
Expand Down Expand Up @@ -418,6 +426,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
arg: GenericArg<'tcx>,
error_code: TypeAnnotationNeeded,
should_label_span: bool,
param_env: ty::ParamEnv<'tcx>,
originating_projection: Option<ty::ProjectionPredicate<'tcx>>,
) -> Diag<'a> {
let arg = self.resolve_vars_if_possible(arg);
let arg_data = self.extract_inference_diagnostics_data(arg, None);
Expand Down Expand Up @@ -451,17 +461,56 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
let mut infer_subdiags = Vec::new();
let mut multi_suggestions = Vec::new();
match kind {
InferSourceKind::LetBinding { insert_span, pattern_name, ty, def_id } => {
infer_subdiags.push(SourceKindSubdiag::LetLike {
span: insert_span,
name: pattern_name.map(|name| name.to_string()).unwrap_or_else(String::new),
x_kind: arg_data.where_x_is_kind(ty),
prefix_kind: arg_data.kind.clone(),
prefix: arg_data.kind.try_get_prefix().unwrap_or_default(),
arg_name: arg_data.name,
kind: if pattern_name.is_some() { "with_pattern" } else { "other" },
type_name: ty_to_string(self, ty, def_id),
});
InferSourceKind::LetBinding {
insert_span,
pattern_name,
ty,
def_id,
init_expr_hir_id,
} => {
let mut paths = vec![];
if let Some(def_id) = def_id
&& let Some(hir_id) = init_expr_hir_id
&& let expr = self.infcx.tcx.hir().expect_expr(hir_id)
&& let hir::ExprKind::MethodCall(_, rcvr, _, _) = expr.kind
&& let Some(ty) = typeck_results.node_type_opt(rcvr.hir_id)
{
paths = self.get_fully_qualified_path_suggestions_from_impls(
ty,
def_id,
InferenceSuggestionFormat::BindingType,
param_env,
originating_projection,
);
}

if paths.is_empty() {
infer_subdiags.push(SourceKindSubdiag::LetLike {
span: insert_span,
name: pattern_name.map(|name| name.to_string()).unwrap_or_else(String::new),
x_kind: arg_data.where_x_is_kind(ty),
prefix_kind: arg_data.kind.clone(),
prefix: arg_data.kind.try_get_prefix().unwrap_or_default(),
arg_name: arg_data.name,
kind: if pattern_name.is_some() { "with_pattern" } else { "other" },
type_name: ty_to_string(self, ty, def_id),
});
} else {
for type_name in paths {
infer_subdiags.push(SourceKindSubdiag::LetLike {
span: insert_span,
name: pattern_name
.map(|name| name.to_string())
.unwrap_or_else(String::new),
x_kind: arg_data.where_x_is_kind(ty),
prefix_kind: arg_data.kind.clone(),
prefix: arg_data.kind.try_get_prefix().unwrap_or_default(),
arg_name: arg_data.name.clone(),
kind: if pattern_name.is_some() { "with_pattern" } else { "other" },
type_name,
});
}
}
}
InferSourceKind::ClosureArg { insert_span, ty } => {
infer_subdiags.push(SourceKindSubdiag::LetLike {
Expand Down Expand Up @@ -557,12 +606,35 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
_ => "",
};

multi_suggestions.push(SourceKindMultiSuggestion::new_fully_qualified(
receiver.span,
def_path,
adjustment,
successor,
));
// Look for all the possible implementations to suggest, otherwise we'll show
// just suggest the syntax for the fully qualified path with placeholders.
let paths = self.get_fully_qualified_path_suggestions_from_impls(
args.type_at(0),
def_id,
InferenceSuggestionFormat::FullyQualifiedMethodCall,
param_env,
originating_projection,
);
if paths.len() > 20 || paths.is_empty() {
// This will show the fallback impl, so the expression will have type
// parameter placeholders, but it's better than nothing.
multi_suggestions.push(SourceKindMultiSuggestion::new_fully_qualified(
receiver.span,
def_path,
adjustment,
successor,
));
} else {
// These are the paths to specific impls.
for path in paths {
multi_suggestions.push(SourceKindMultiSuggestion::new_fully_qualified(
receiver.span,
path,
adjustment,
successor,
));
}
}
}
}
InferSourceKind::ClosureReturn { ty, data, should_wrap_expr } => {
Expand Down Expand Up @@ -613,6 +685,122 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
}),
}
}

fn get_fully_qualified_path_suggestions_from_impls(
&self,
self_ty: Ty<'tcx>,
def_id: DefId,
target_type: InferenceSuggestionFormat,
param_env: ty::ParamEnv<'tcx>,
originating_projection: Option<ty::ProjectionPredicate<'tcx>>,
) -> Vec<String> {
let tcx = self.infcx.tcx;
let mut paths = vec![];
let name = tcx.item_name(def_id);
let trait_def_id = tcx.parent(def_id);
tcx.for_each_relevant_impl(trait_def_id, self_ty, |impl_def_id| {
let impl_args = self.fresh_args_for_item(DUMMY_SP, impl_def_id);
let impl_trait_ref =
tcx.impl_trait_ref(impl_def_id).unwrap().instantiate(tcx, impl_args);
let impl_self_ty = impl_trait_ref.self_ty();
if self.infcx.can_eq(param_env, impl_self_ty, self_ty) {
// The expr's self type could conform to this impl's self type.
} else {
// Nope, don't bother.
return;
}

let filter = if let Some(ty::ProjectionPredicate {
projection_term: ty::AliasTerm { def_id, .. },
term,
}) = originating_projection
&& let ty::TermKind::Ty(assoc_ty) = term.unpack()
&& tcx.item_name(def_id) == sym::Output
&& hir::lang_items::BINARY_OPERATORS
.iter()
.map(|&op| tcx.lang_items().get(op))
.any(|op| op == Some(tcx.parent(def_id)))
{
// If the predicate that failed to be inferred is an associated type called
// "Output" (from one of the math traits), we will only mention the `Into` and
// `From` impls that correspond to the self type as well, so as to avoid showing
// multiple conversion options.
Some(assoc_ty)
} else {
None
};
let assocs = tcx.associated_items(impl_def_id);

if tcx.is_diagnostic_item(sym::blanket_into_impl, impl_def_id)
&& let Some(did) = tcx.get_diagnostic_item(sym::From)
{
let mut found = false;
tcx.for_each_impl(did, |impl_def_id| {
// We had an `<A as Into<B>::into` and we've hit the blanket
// impl for `From<A>`. So we try and look for the right `From`
// impls that *would* apply. We *could* do this in a generalized
// version by evaluating the `where` clauses, but that would be
// way too involved to implement. Instead we special case the
// arguably most common case of `expr.into()`.
let Some(header) = tcx.impl_trait_header(impl_def_id) else {
return;
};
let target = header.trait_ref.skip_binder().args.type_at(0);
if filter.is_some() && filter != Some(target) {
return;
};
let target = header.trait_ref.skip_binder().args.type_at(0);
let ty = header.trait_ref.skip_binder().args.type_at(1);
if ty == self_ty {
match target_type {
InferenceSuggestionFormat::BindingType => {
paths.push(if let ty::Infer(_) = target.kind() {
"/* Type */".to_string()
} else {
format!("{target}")
});
}
InferenceSuggestionFormat::FullyQualifiedMethodCall => {
paths.push(format!("<{self_ty} as Into<{target}>>::into"));
}
}
found = true;
}
});
if found {
return;
}
}

// We're at the `impl` level, but we want to get the same method we
// called *on this `impl`*, in order to get the right DefId and args.
let Some(assoc) = assocs.filter_by_name_unhygienic(name).next() else {
// The method isn't in this `impl`? Not useful to us then.
return;
};
let Some(trait_assoc_item) = assoc.trait_item_def_id else {
return;
};
let args = impl_trait_ref
.args
.extend_to(tcx, trait_assoc_item, |def, _| self.var_for_def(DUMMY_SP, def));
match target_type {
InferenceSuggestionFormat::BindingType => {
let fn_sig = tcx.fn_sig(def_id).instantiate(tcx, args);
let ret = fn_sig.skip_binder().output();
paths.push(if let ty::Infer(_) = ret.kind() {
"/* Type */".to_string()
} else {
format!("{ret}")
});
}
InferenceSuggestionFormat::FullyQualifiedMethodCall => {
paths.push(self.tcx.value_path_str_with_args(def_id, args));
}
}
});
paths
}
}

#[derive(Debug)]
Expand All @@ -628,6 +816,7 @@ enum InferSourceKind<'tcx> {
pattern_name: Option<Ident>,
ty: Ty<'tcx>,
def_id: Option<DefId>,
init_expr_hir_id: Option<HirId>,
},
ClosureArg {
insert_span: Span,
Expand Down Expand Up @@ -813,8 +1002,11 @@ impl<'a, 'tcx> FindInferSourceVisitor<'a, 'tcx> {
let cost = self.source_cost(&new_source) + self.attempt;
debug!(?cost);
self.attempt += 1;
if let Some(InferSource { kind: InferSourceKind::GenericArg { def_id: did, .. }, .. }) =
self.infer_source
if let Some(InferSource { kind: InferSourceKind::GenericArg { def_id: did, .. }, .. })
| Some(InferSource {
kind: InferSourceKind::FullyQualifiedMethodCall { def_id: did, .. },
..
}) = self.infer_source
&& let InferSourceKind::LetBinding { ref ty, ref mut def_id, .. } = new_source.kind
&& ty.is_ty_or_numeric_infer()
{
Expand Down Expand Up @@ -1123,6 +1315,7 @@ impl<'a, 'tcx> Visitor<'tcx> for FindInferSourceVisitor<'a, 'tcx> {
pattern_name: local.pat.simple_ident(),
ty,
def_id: None,
init_expr_hir_id: local.init.map(|e| e.hir_id),
},
})
}
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_trait_selection/src/error_reporting/traits/ambiguity.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
trait_ref.self_ty().skip_binder().into(),
TypeAnnotationNeeded::E0282,
false,
obligation.param_env,
None,
);
return err.stash(span, StashKey::MaybeForgetReturn).unwrap();
}
Expand Down Expand Up @@ -237,6 +239,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
arg,
TypeAnnotationNeeded::E0283,
true,
obligation.param_env,
None,
)
} else {
struct_span_code_err!(
Expand Down Expand Up @@ -477,6 +481,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
arg,
TypeAnnotationNeeded::E0282,
false,
obligation.param_env,
None,
)
}

Expand All @@ -496,6 +502,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
a.into(),
TypeAnnotationNeeded::E0282,
true,
obligation.param_env,
None,
)
}
ty::PredicateKind::Clause(ty::ClauseKind::Projection(data)) => {
Expand Down Expand Up @@ -526,6 +534,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
arg,
TypeAnnotationNeeded::E0284,
true,
obligation.param_env,
Some(data),
)
.with_note(format!("cannot satisfy `{predicate}`"))
} else {
Expand Down Expand Up @@ -556,6 +566,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
arg,
TypeAnnotationNeeded::E0284,
true,
obligation.param_env,
None,
);
err
} else {
Expand All @@ -578,6 +590,8 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> {
ct.into(),
TypeAnnotationNeeded::E0284,
true,
obligation.param_env,
None,
),
ty::PredicateKind::NormalizesTo(ty::NormalizesTo { alias, term })
if term.is_infer() =>
Expand Down
Loading

0 comments on commit adc3ea6

Please sign in to comment.