Skip to content

Commit

Permalink
Track HirId instead of Span in `ObligationCauseCode::SizedArgumen…
Browse files Browse the repository at this point in the history
…tType`

This gets us more accurate suggestions.
  • Loading branch information
estebank committed Dec 20, 2023
1 parent fd1006e commit 9528085
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 81 deletions.
7 changes: 4 additions & 3 deletions compiler/rustc_hir_typeck/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,21 +115,22 @@ pub(super) fn check_fn<'a, 'tcx>(
let inputs_fn = fn_sig.inputs().iter().copied();
for (idx, (param_ty, param)) in inputs_fn.chain(maybe_va_list).zip(body.params).enumerate() {
// Check the pattern.
let ty_span = try { inputs_hir?.get(idx)?.span };
let ty: Option<&hir::Ty<'_>> = try { inputs_hir?.get(idx)? };
let ty_span = ty.map(|ty| ty.span);
fcx.check_pat_top(param.pat, param_ty, ty_span, None, None);

// Check that argument is Sized.
if !params_can_be_unsized {
fcx.require_type_is_sized(
param_ty,
param.pat.span,
// ty_span == binding_span iff this is a closure parameter with no type ascription,
// ty.span == binding_span iff this is a closure parameter with no type ascription,
// or if it's an implicit `self` parameter
traits::SizedArgumentType(
if ty_span == Some(param.span) && tcx.is_closure(fn_def_id.into()) {
None
} else {
ty_span
ty.map(|ty| ty.hir_id)
},
),
);
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_hir_typeck/src/gather_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub(super) struct GatherLocalsVisitor<'a, 'tcx> {
// parameters are special cases of patterns, but we want to handle them as
// *distinct* cases. so track when we are hitting a pattern *within* an fn
// parameter.
outermost_fn_param_pat: Option<Span>,
outermost_fn_param_pat: Option<(Span, hir::HirId)>,
}

impl<'a, 'tcx> GatherLocalsVisitor<'a, 'tcx> {
Expand Down Expand Up @@ -131,7 +131,8 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
}

fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) {
let old_outermost_fn_param_pat = self.outermost_fn_param_pat.replace(param.ty_span);
let old_outermost_fn_param_pat =
self.outermost_fn_param_pat.replace((param.ty_span, param.hir_id));
intravisit::walk_param(self, param);
self.outermost_fn_param_pat = old_outermost_fn_param_pat;
}
Expand All @@ -141,7 +142,7 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
if let PatKind::Binding(_, _, ident, _) = p.kind {
let var_ty = self.assign(p.span, p.hir_id, None);

if let Some(ty_span) = self.outermost_fn_param_pat {
if let Some((ty_span, hir_id)) = self.outermost_fn_param_pat {
if !self.fcx.tcx.features().unsized_fn_params {
self.fcx.require_type_is_sized(
var_ty,
Expand All @@ -154,7 +155,7 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
{
None
} else {
Some(ty_span)
Some(hir_id)
},
),
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ pub enum ObligationCauseCode<'tcx> {
/// Type of each variable must be `Sized`.
VariableType(hir::HirId),
/// Argument type must be `Sized`.
SizedArgumentType(Option<Span>),
SizedArgumentType(Option<hir::HirId>),
/// Return type must be `Sized`.
SizedReturnType,
/// Yield type must be `Sized`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ use rustc_errors::{
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::Visitor;
use rustc_hir::intravisit::{Map, Visitor};
use rustc_hir::is_range_literal;
use rustc_hir::lang_items::LangItem;
use rustc_hir::{CoroutineKind, CoroutineSource, Node};
use rustc_hir::{Expr, HirId};
use rustc_hir::{CoroutineKind, CoroutineSource, Expr, HirId, Node};
use rustc_infer::infer::error_reporting::TypeErrCtxt;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
use rustc_infer::infer::{BoundRegionConversionTime, DefineOpaqueTypes, InferOk};
Expand Down Expand Up @@ -3029,63 +3028,80 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
err.help("unsized locals are gated as an unstable feature");
}
}
ObligationCauseCode::SizedArgumentType(ty_span) => {
if let Some(span) = ty_span {
let trait_len = if let ty::PredicateKind::Clause(clause) =
predicate.kind().skip_binder()
&& let ty::ClauseKind::Trait(trait_pred) = clause
&& let ty::Dynamic(preds, ..) = trait_pred.self_ty().kind()
{
let span = if let Ok(snippet) =
self.tcx.sess.source_map().span_to_snippet(span)
&& snippet.starts_with("dyn ")
{
let pos = snippet.len() - snippet[3..].trim_start().len();
span.with_hi(span.lo() + BytePos(pos as u32))
} else {
span.shrink_to_lo()
};
err.span_suggestion_verbose(
span,
"you can use `impl Trait` as the argument type",
"impl ".to_string(),
Applicability::MaybeIncorrect,
);
preds
.iter()
.filter(|pred| {
// We only want to count `dyn Foo + Bar`, not `dyn Foo<Bar>`,
// because the later doesn't need parentheses.
matches!(
pred.skip_binder(),
ty::ExistentialPredicate::Trait(_)
| ty::ExistentialPredicate::AutoTrait(_)
)
})
.count()
} else {
1
};
let sugg = if trait_len == 1 {
vec![(span.shrink_to_lo(), "&".to_string())]
} else if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span)
&& snippet.starts_with('(')
{
// We don't want to suggest `&((dyn Foo + Bar))` when we have
// `(dyn Foo + Bar)`.
vec![(span.shrink_to_lo(), "&".to_string())]
} else {
vec![
(span.shrink_to_lo(), "&(".to_string()),
(span.shrink_to_hi(), ")".to_string()),
]
};
err.multipart_suggestion_verbose(
"function arguments must have a statically known size, borrowed types \
always have a known size",
sugg,
Applicability::MachineApplicable,
);
ObligationCauseCode::SizedArgumentType(hir_id) => {
let mut ty = None;
let borrowed_msg = "function arguments must have a statically known size, borrowed \
types always have a known size";
if let Some(hir_id) = hir_id
&& let Some(hir::Node::Param(param)) = self.tcx.hir().find(hir_id)
&& let Some(item) = self.tcx.hir().find_parent(hir_id)
&& let Some(decl) = item.fn_decl()
&& let Some(t) = decl.inputs.iter().find(|t| param.ty_span.contains(t.span))
{
// We use `contains` because the type might be surrounded by parentheses,
// which makes `ty_span` and `t.span` disagree with each other, but one
// fully contains the other: `foo: (dyn Foo + Bar)`
// ^-------------^
// ||
// |t.span
// param._ty_span
ty = Some(t);
} else if let Some(hir_id) = hir_id
&& let Some(hir::Node::Ty(t)) = self.tcx.hir().find(hir_id)
{
ty = Some(t);
}
if let Some(ty) = ty {
match ty.kind {
hir::TyKind::TraitObject(traits, _, _) => {
let (span, kw) = match traits {
[first, ..] if first.span.lo() == ty.span.lo() => {
// Missing `dyn` in front of trait object.
(ty.span.shrink_to_lo(), "dyn ")
}
[first, ..] => (ty.span.until(first.span), ""),
[] => span_bug!(ty.span, "trait object with no traits: {ty:?}"),
};
let needs_parens = traits.len() != 1;
err.span_suggestion_verbose(
span,
"you can use `impl Trait` as the argument type",
"impl ".to_string(),
Applicability::MaybeIncorrect,
);
let sugg = if !needs_parens {
vec![(span.shrink_to_lo(), format!("&{kw}"))]
} else {
vec![
(span.shrink_to_lo(), format!("&({kw}")),
(ty.span.shrink_to_hi(), ")".to_string()),
]
};
err.multipart_suggestion_verbose(
borrowed_msg,
sugg,
Applicability::MachineApplicable,
);
}
hir::TyKind::Slice(_ty) => {
err.span_suggestion_verbose(
ty.span.shrink_to_lo(),
"function arguments must have a statically known size, borrowed \
slices always have a known size",
"&",
Applicability::MachineApplicable,
);
}
hir::TyKind::Path(_) => {
err.span_suggestion_verbose(
ty.span.shrink_to_lo(),
borrowed_msg,
"&",
Applicability::MachineApplicable,
);
}
_ => {}
}
} else {
err.note("all function arguments must have a statically known size");
}
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/feature-gates/feature-gate-unsized_fn_params.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ LL | fn bar(x: impl Foo) {
| ++++
help: function arguments must have a statically known size, borrowed types always have a known size
|
LL | fn bar(x: &Foo) {
| +
LL | fn bar(x: &dyn Foo) {
| ++++

error[E0277]: the size for values of type `[()]` cannot be known at compilation time
--> $DIR/feature-gate-unsized_fn_params.rs:25:8
Expand All @@ -40,7 +40,7 @@ LL | fn qux(_: [()]) {}
|
= help: the trait `Sized` is not implemented for `[()]`
= help: unsized fn params are gated as an unstable feature
help: function arguments must have a statically known size, borrowed types always have a known size
help: function arguments must have a statically known size, borrowed slices always have a known size
|
LL | fn qux(_: &[()]) {}
| +
Expand Down
4 changes: 0 additions & 4 deletions tests/ui/resolve/issue-5035-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@ LL | fn foo(_x: K) {}
|
= help: the trait `Sized` is not implemented for `(dyn I + 'static)`
= help: unsized fn params are gated as an unstable feature
help: you can use `impl Trait` as the argument type
|
LL | fn foo(_x: impl K) {}
| ++++
help: function arguments must have a statically known size, borrowed types always have a known size
|
LL | fn foo(_x: &K) {}
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/traits/bound/not-on-bare-trait.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ LL | fn foo(_x: impl Foo + Send) {
| ++++
help: function arguments must have a statically known size, borrowed types always have a known size
|
LL | fn foo(_x: &(Foo + Send)) {
| ++ +
LL | fn foo(_x: &(dyn Foo + Send)) {
| +++++ +

error[E0277]: the size for values of type `(dyn Foo + Send + 'static)` cannot be known at compilation time
--> $DIR/not-on-bare-trait.rs:12:8
Expand All @@ -47,12 +47,12 @@ LL | fn bar(_x: (dyn Foo + Send)) {
= help: unsized fn params are gated as an unstable feature
help: you can use `impl Trait` as the argument type
|
LL | fn bar(_x: impl (dyn Foo + Send)) {
| ++++
LL | fn bar(_x: (impl Foo + Send)) {
| ~~~~
help: function arguments must have a statically known size, borrowed types always have a known size
|
LL | fn bar(_x: &(dyn Foo + Send)) {
| +
LL | fn bar(_x: (&(dyn Foo + Send))) {
| ++ +

error: aborting due to 2 previous errors; 1 warning emitted

Expand Down

0 comments on commit 9528085

Please sign in to comment.