Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Standardize "use parentheses to call" suggestions between typeck and trait selection #102863

Merged
merged 5 commits into from
Oct 19, 2022
Merged

Standardize "use parentheses to call" suggestions between typeck and trait selection #102863

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
63be7a2
Suggest calling ctor when trait is unimplemented
compiler-errors Oct 9, 2022
b3edd9f
Use predicate_must_hold_modulo_regions
compiler-errors Oct 9, 2022
f5336a9
Standardize arg suggestions between typeck and trait selection
compiler-errors Oct 9, 2022
7eb2d4e
Generalize call suggestion for unsatisfied predicate
compiler-errors Oct 9, 2022
35f1570
instantiate -> construct
compiler-errors Oct 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion compiler/rustc_hir_analysis/src/check/callee.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::method::probe::{IsSuggestion, Mode, ProbeScope};
use super::method::MethodCallee;
use super::{DefIdOrName, Expectation, FnCtxt, TupleArgumentsFlag};
use super::{Expectation, FnCtxt, TupleArgumentsFlag};
use crate::type_error_struct;

use rustc_ast::util::parser::PREC_POSTFIX;
Expand All @@ -27,6 +27,7 @@ use rustc_span::Span;
use rustc_target::spec::abi;
use rustc_trait_selection::autoderef::Autoderef;
use rustc_trait_selection::infer::InferCtxtExt as _;
use rustc_trait_selection::traits::error_reporting::DefIdOrName;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;

use std::iter;
Expand Down
15 changes: 4 additions & 11 deletions compiler/rustc_hir_analysis/src/check/fn_ctxt/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use super::FnCtxt;
use crate::astconv::AstConv;
use crate::errors::{AddReturnTypeSuggestion, ExpectedReturnTypeLabel};

use hir::def_id::DefId;
use rustc_ast::util::parser::{ExprPrecedence, PREC_POSTFIX};
use rustc_errors::{Applicability, Diagnostic, MultiSpan};
use rustc_hir as hir;
Expand All @@ -19,6 +18,7 @@ use rustc_session::errors::ExprParenthesesNeeded;
use rustc_span::symbol::sym;
use rustc_span::Span;
use rustc_trait_selection::infer::InferCtxtExt;
use rustc_trait_selection::traits::error_reporting::DefIdOrName;
use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _;

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -90,7 +90,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if ty.is_suggestable(self.tcx, false) {
format!("/* {ty} */")
} else {
"".to_string()
"/* value */".to_string()
}
})
.collect::<Vec<_>>()
Expand All @@ -102,10 +102,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

let msg = match def_id_or_name {
DefIdOrName::DefId(def_id) => match self.tcx.def_kind(def_id) {
DefKind::Ctor(CtorOf::Struct, _) => "instantiate this tuple struct".to_string(),
DefKind::Ctor(CtorOf::Variant, _) => {
"instantiate this tuple variant".to_string()
}
DefKind::Ctor(CtorOf::Struct, _) => "construct this tuple struct".to_string(),
DefKind::Ctor(CtorOf::Variant, _) => "construct this tuple variant".to_string(),
kind => format!("call this {}", kind.descr(def_id)),
},
DefIdOrName::Name(name) => format!("call this {name}"),
Expand Down Expand Up @@ -1209,8 +1207,3 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
}

pub enum DefIdOrName {
DefId(DefId),
Name(&'static str),
}
13 changes: 9 additions & 4 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@ pub mod on_unimplemented;
pub mod suggestions;

use super::{
EvaluationResult, FulfillmentContext, FulfillmentError, FulfillmentErrorCode,
MismatchedProjectionTypes, Obligation, ObligationCause, ObligationCauseCode,
OnUnimplementedDirective, OnUnimplementedNote, OutputTypeParameterMismatch, Overflow,
PredicateObligation, SelectionContext, SelectionError, TraitNotObjectSafe,
FulfillmentContext, FulfillmentError, FulfillmentErrorCode, MismatchedProjectionTypes,
Obligation, ObligationCause, ObligationCauseCode, OnUnimplementedDirective,
OnUnimplementedNote, OutputTypeParameterMismatch, Overflow, PredicateObligation,
SelectionContext, SelectionError, TraitNotObjectSafe,
};

use crate::infer::error_reporting::{TyCategory, TypeAnnotationNeeded as ErrorCode};
Expand Down Expand Up @@ -2796,3 +2796,8 @@ impl<'tcx> ty::TypeVisitor<'tcx> for HasNumericInferVisitor {
}
}
}

pub enum DefIdOrName {
DefId(DefId),
Name(&'static str),
}
217 changes: 153 additions & 64 deletions compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use super::{
EvaluationResult, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation,
DefIdOrName, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation,
SelectionContext,
};

use crate::autoderef::Autoderef;
use crate::infer::InferCtxt;
use crate::traits::normalize_to;

use hir::def::CtorOf;
use hir::HirId;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::stack::ensure_sufficient_stack;
Expand All @@ -22,14 +23,15 @@ use rustc_hir::lang_items::LangItem;
use rustc_hir::{AsyncGeneratorKind, GeneratorKind, Node};
use rustc_infer::infer::error_reporting::TypeErrCtxt;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::LateBoundRegionConversionTime;
use rustc_middle::hir::map;
use rustc_middle::ty::{
self, suggest_arbitrary_trait_bound, suggest_constraining_type_param, AdtKind, DefIdTree,
GeneratorDiagnosticData, GeneratorInteriorTypeCause, Infer, InferTy, IsSuggestable,
ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable, TypeVisitable,
};
use rustc_middle::ty::{TypeAndMut, TypeckResults};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{BytePos, DesugaringKind, ExpnKind, Span, DUMMY_SP};
use rustc_target::spec::abi;
use std::fmt;
Expand Down Expand Up @@ -812,74 +814,136 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
err: &mut Diagnostic,
trait_pred: ty::PolyTraitPredicate<'tcx>,
) -> bool {
// Skipping binder here, remapping below
let self_ty = trait_pred.self_ty().skip_binder();

let (def_id, output_ty, callable) = match *self_ty.kind() {
ty::Closure(def_id, substs) => (def_id, substs.as_closure().sig().output(), "closure"),
ty::FnDef(def_id, _) => (def_id, self_ty.fn_sig(self.tcx).output(), "function"),
_ => return false,
};
let msg = format!("use parentheses to call the {}", callable);
if let ty::PredicateKind::Trait(trait_pred) = obligation.predicate.kind().skip_binder()
&& Some(trait_pred.def_id()) == self.tcx.lang_items().sized_trait()
{
// Don't suggest calling to turn an unsized type into a sized type
return false;
}

// "We should really create a single list of bound vars from the combined vars
// from the predicate and function, but instead we just liberate the function bound vars"
let output_ty = self.tcx.liberate_late_bound_regions(def_id, output_ty);
// This is duplicated from `extract_callable_info` in typeck, which
// relies on autoderef, so we can't use it here.
let found = trait_pred.self_ty().skip_binder().peel_refs();
let Some((def_id_or_name, output, inputs)) = (match *found.kind()
{
ty::FnPtr(fn_sig) => {
Some((DefIdOrName::Name("function pointer"), fn_sig.output(), fn_sig.inputs()))
}
ty::FnDef(def_id, _) => {
let fn_sig = found.fn_sig(self.tcx);
Some((DefIdOrName::DefId(def_id), fn_sig.output(), fn_sig.inputs()))
}
ty::Closure(def_id, substs) => {
let fn_sig = substs.as_closure().sig();
Some((
DefIdOrName::DefId(def_id),
fn_sig.output(),
fn_sig.inputs().map_bound(|inputs| &inputs[1..]),
))
}
ty::Opaque(def_id, substs) => {
self.tcx.bound_item_bounds(def_id).subst(self.tcx, substs).iter().find_map(|pred| {
if let ty::PredicateKind::Projection(proj) = pred.kind().skip_binder()
&& Some(proj.projection_ty.item_def_id) == self.tcx.lang_items().fn_once_output()
// args tuple will always be substs[1]
&& let ty::Tuple(args) = proj.projection_ty.substs.type_at(1).kind()
{
Some((
DefIdOrName::DefId(def_id),
pred.kind().rebind(proj.term.ty().unwrap()),
pred.kind().rebind(args.as_slice()),
))
} else {
None
}
})
}
ty::Dynamic(data, _, ty::Dyn) => {
data.iter().find_map(|pred| {
if let ty::ExistentialPredicate::Projection(proj) = pred.skip_binder()
&& Some(proj.item_def_id) == self.tcx.lang_items().fn_once_output()
// for existential projection, substs are shifted over by 1
&& let ty::Tuple(args) = proj.substs.type_at(0).kind()
{
Some((
DefIdOrName::Name("trait object"),
pred.rebind(proj.term.ty().unwrap()),
pred.rebind(args.as_slice()),
))
} else {
None
}
})
}
ty::Param(_) => {
obligation.param_env.caller_bounds().iter().find_map(|pred| {
if let ty::PredicateKind::Projection(proj) = pred.kind().skip_binder()
&& Some(proj.projection_ty.item_def_id) == self.tcx.lang_items().fn_once_output()
&& proj.projection_ty.self_ty() == found
// args tuple will always be substs[1]
&& let ty::Tuple(args) = proj.projection_ty.substs.type_at(1).kind()
{
Some((
DefIdOrName::Name("type parameter"),
pred.kind().rebind(proj.term.ty().unwrap()),
pred.kind().rebind(args.as_slice()),
))
} else {
None
}
})
}
_ => None,
}) else { return false; };
let output = self.replace_bound_vars_with_fresh_vars(
obligation.cause.span,
LateBoundRegionConversionTime::FnCall,
output,
);
let inputs = inputs.skip_binder().iter().map(|ty| {
self.replace_bound_vars_with_fresh_vars(
obligation.cause.span,
LateBoundRegionConversionTime::FnCall,
inputs.rebind(*ty),
)
});

// Remapping bound vars here
let trait_pred_and_self = trait_pred.map_bound(|trait_pred| (trait_pred, output_ty));
let trait_pred_and_self = trait_pred.map_bound(|trait_pred| (trait_pred, output));

let new_obligation =
self.mk_trait_obligation_with_new_self_ty(obligation.param_env, trait_pred_and_self);

match self.evaluate_obligation(&new_obligation) {
Ok(
EvaluationResult::EvaluatedToOk
| EvaluationResult::EvaluatedToOkModuloRegions
| EvaluationResult::EvaluatedToOkModuloOpaqueTypes
| EvaluationResult::EvaluatedToAmbig,
) => {}
_ => return false,
if !self.predicate_must_hold_modulo_regions(&new_obligation) {
return false;
}
let hir = self.tcx.hir();

// Get the name of the callable and the arguments to be used in the suggestion.
let (snippet, sugg) = match hir.get_if_local(def_id) {
Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Closure(hir::Closure { fn_decl, fn_decl_span, .. }),
..
})) => {
err.span_label(*fn_decl_span, "consider calling this closure");
let Some(name) = self.get_closure_name(def_id, err, &msg) else {
return false;
};
let args = fn_decl.inputs.iter().map(|_| "_").collect::<Vec<_>>().join(", ");
let sugg = format!("({})", args);
(format!("{}{}", name, sugg), sugg)
}
Some(hir::Node::Item(hir::Item {
ident,
kind: hir::ItemKind::Fn(.., body_id),
..
})) => {
err.span_label(ident.span, "consider calling this function");
let body = hir.body(*body_id);
let args = body
.params
.iter()
.map(|arg| match &arg.pat.kind {
hir::PatKind::Binding(_, _, ident, None)
// FIXME: provide a better suggestion when encountering `SelfLower`, it
// should suggest a method call.
if ident.name != kw::SelfLower => ident.to_string(),
_ => "_".to_string(),
})
.collect::<Vec<_>>()
.join(", ");
let sugg = format!("({})", args);
(format!("{}{}", ident, sugg), sugg)
}
_ => return false,
let hir = self.tcx.hir();

let msg = match def_id_or_name {
DefIdOrName::DefId(def_id) => match self.tcx.def_kind(def_id) {
DefKind::Ctor(CtorOf::Struct, _) => {
"use parentheses to construct this tuple struct".to_string()
}
DefKind::Ctor(CtorOf::Variant, _) => {
"use parentheses to construct this tuple variant".to_string()
}
kind => format!("use parentheses to call this {}", kind.descr(def_id)),
},
DefIdOrName::Name(name) => format!("use parentheses to call this {name}"),
};

let args = inputs
.map(|ty| {
if ty.is_suggestable(self.tcx, false) {
format!("/* {ty} */")
} else {
"/* value */".to_string()
}
})
.collect::<Vec<_>>()
.join(", ");

if matches!(obligation.cause.code(), ObligationCauseCode::FunctionArgumentObligation { .. })
&& obligation.cause.span.can_be_used_for_suggestions()
{
Expand All @@ -890,11 +954,36 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
err.span_suggestion_verbose(
obligation.cause.span.shrink_to_hi(),
&msg,
sugg,
format!("({args})"),
Applicability::HasPlaceholders,
);
} else {
err.help(&format!("{}: `{}`", msg, snippet));
} else if let DefIdOrName::DefId(def_id) = def_id_or_name {
let name = match hir.get_if_local(def_id) {
Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Closure(hir::Closure { fn_decl_span, .. }),
..
})) => {
err.span_label(*fn_decl_span, "consider calling this closure");
let Some(name) = self.get_closure_name(def_id, err, &msg) else {
return false;
};
name.to_string()
}
Some(hir::Node::Item(hir::Item { ident, kind: hir::ItemKind::Fn(..), .. })) => {
err.span_label(ident.span, "consider calling this function");
ident.to_string()
}
Some(hir::Node::Ctor(..)) => {
let name = self.tcx.def_path_str(def_id);
err.span_label(
self.tcx.def_span(def_id),
format!("consider calling the constructor for `{}`", name),
);
name
}
_ => return false,
};
err.help(&format!("{msg}: `{name}({args})`"));
}
true
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/binop/issue-77910-1.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ LL | assert_eq!(foo, y);
| ^^^^^^^^^^^^^^^^^^ `for<'a> fn(&'a i32) -> &'a i32 {foo}` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
= help: the trait `Debug` is not implemented for fn item `for<'a> fn(&'a i32) -> &'a i32 {foo}`
= help: use parentheses to call the function: `foo(s)`
= help: use parentheses to call this function: `foo(/* &i32 */)`
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/closures/closure-bounds-subtype.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ note: required by a bound in `take_const_owned`
|
LL | fn take_const_owned<F>(_: F) where F: FnOnce() + Sync + Send {
| ^^^^ required by this bound in `take_const_owned`
help: use parentheses to call this type parameter
|
LL | take_const_owned(f());
Copy link
Member Author

@compiler-errors compiler-errors Oct 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is, unfortunately, an erroneous suggestion that is uncovered by the generalizations I made in the last commit in the stack. However, this is not really the fault of the changes I made. For example, a similarly erroneous suggestion already triggers on nightly with this code:

struct Foo;

fn foo() -> Foo {
    todo!()
}

trait Bar {}

impl Bar for Foo {}

fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }

fn main() {
    needs_foo_and_callable(foo);
}

Results in:

error[E0277]: the trait bound `fn() -> Foo {foo}: Bar` is not satisfied
  --> src/main.rs:14:15
   |
3  | fn foo() -> Foo {
   |    --- consider calling this function
...
14 |     needs_foo_and_callable(foo);
   |     ---------------------- ^^^ the trait `Bar` is not implemented for fn item `fn() -> Foo {foo}`
   |     |
   |     required by a bound introduced by this call
   |
note: required by a bound in `needs_foo_and_callable`
  --> src/main.rs:11:25
   |
11 | fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }
   |                                      ^^^ required by this bound in `needs_foo`
help: use parentheses to call the function
   |
14 |     needs_foo_and_callable(foo());
   |                               ++

But adding those parentheses causes:

error[E0277]: expected a `Fn<()>` closure, found `Foo`
  --> src/main.rs:14:15
   |
14 |     needs_foo_and_callable(foo());
   |     ---------------------- ^^^^^ expected an `Fn<()>` closure, found `Foo`
   |     |
   |     required by a bound introduced by this call
   |
   = help: the trait `Fn<()>` is not implemented for `Foo`
   = note: wrap the `Foo` in a closure with no arguments: `|| { /* code */ }`
note: required by a bound in `needs_foo_and_callable`
  --> src/main.rs:11:31
   |
11 | fn needs_foo_and_callable<T>(_: impl Bar + Fn() -> T) { }
   |                                            ^^^^^^^^^ required by this bound in `needs_foo`

For more information about this error, try `rustc --explain E0277`.

This is a fundamental limitation to the suggestion being implemented here in trait selection -- I'm happy to brainstorm ways of suppressing this, but I'm happy with keeping it as is and filing a follow-up bug.

| ++
help: consider further restricting this bound
|
LL | fn give_owned<F>(f: F) where F: FnOnce() + Send + std::marker::Sync {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-35241.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ LL | fn test() -> Foo { Foo }
|
= note: expected struct `Foo`
found fn item `fn(u32) -> Foo {Foo}`
help: use parentheses to instantiate this tuple struct
help: use parentheses to construct this tuple struct
|
LL | fn test() -> Foo { Foo(/* u32 */) }
| +++++++++++
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-70724-add_type_neq_err_label-unwrap.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ LL | assert_eq!(a, 0);
| ^^^^^^^^^^^^^^^^ `fn() -> i32 {a}` cannot be formatted using `{:?}` because it doesn't implement `Debug`
|
= help: the trait `Debug` is not implemented for fn item `fn() -> i32 {a}`
= help: use parentheses to call the function: `a()`
= help: use parentheses to call this function: `a()`
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 3 previous errors
Expand Down
Loading