Skip to content

Commit 5b8bfc1

Browse files
estebankestebank
committed
Track HirId instead of Span in ObligationCauseCode::SizedArgumentType
This gets us more accurate suggestions.
1 parent 5e91e35 commit 5b8bfc1

File tree

8 files changed

+96
-82
lines changed

8 files changed

+96
-82
lines changed

‎compiler/rustc_hir_typeck/src/check.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -115,21 +115,22 @@ pub(super) fn check_fn<'a, 'tcx>(
115115
let inputs_fn = fn_sig.inputs().iter().copied();
116116
for (idx, (param_ty, param)) in inputs_fn.chain(maybe_va_list).zip(body.params).enumerate() {
117117
// Check the pattern.
118-
let ty_span = try { inputs_hir?.get(idx)?.span };
118+
let ty: Option<&hir::Ty<'_>> = try { inputs_hir?.get(idx)? };
119+
let ty_span = ty.map(|ty| ty.span);
119120
fcx.check_pat_top(param.pat, param_ty, ty_span, None, None);
120121

121122
// Check that argument is Sized.
122123
if !params_can_be_unsized {
123124
fcx.require_type_is_sized(
124125
param_ty,
125126
param.pat.span,
126-
// ty_span == binding_span iff this is a closure parameter with no type ascription,
127+
// ty.span == binding_span iff this is a closure parameter with no type ascription,
127128
// or if it's an implicit `self` parameter
128129
traits::SizedArgumentType(
129130
if ty_span == Some(param.span) && tcx.is_closure(fn_def_id.into()) {
130131
None
131132
} else {
132-
ty_span
133+
ty.map(|ty| ty.hir_id)
133134
},
134135
),
135136
);

‎compiler/rustc_hir_typeck/src/gather_locals.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ pub(super) struct GatherLocalsVisitor<'a, 'tcx> {
6060
// parameters are special cases of patterns, but we want to handle them as
6161
// *distinct* cases. so track when we are hitting a pattern *within* an fn
6262
// parameter.
63-
outermost_fn_param_pat: Option<Span>,
63+
outermost_fn_param_pat: Option<(Span, hir::HirId)>,
6464
}
6565

6666
impl<'a, 'tcx> GatherLocalsVisitor<'a, 'tcx> {
@@ -131,7 +131,8 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
131131
}
132132

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

144-
if let Some(ty_span) = self.outermost_fn_param_pat {
145+
if let Some((ty_span, hir_id)) = self.outermost_fn_param_pat {
145146
if !self.fcx.tcx.features().unsized_fn_params {
146147
self.fcx.require_type_is_sized(
147148
var_ty,
@@ -154,7 +155,7 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
154155
{
155156
None
156157
} else {
157-
Some(ty_span)
158+
Some(hir_id)
158159
},
159160
),
160161
);

‎compiler/rustc_middle/src/traits/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ pub enum ObligationCauseCode<'tcx> {
289289
/// Type of each variable must be `Sized`.
290290
VariableType(hir::HirId),
291291
/// Argument type must be `Sized`.
292-
SizedArgumentType(Option<Span>),
292+
SizedArgumentType(Option<hir::HirId>),
293293
/// Return type must be `Sized`.
294294
SizedReturnType,
295295
/// Yield type must be `Sized`.

‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+76-60
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,10 @@ use rustc_errors::{
1919
use rustc_hir as hir;
2020
use rustc_hir::def::{DefKind, Res};
2121
use rustc_hir::def_id::DefId;
22-
use rustc_hir::intravisit::Visitor;
22+
use rustc_hir::intravisit::{Map, Visitor};
2323
use rustc_hir::is_range_literal;
2424
use rustc_hir::lang_items::LangItem;
25-
use rustc_hir::{CoroutineKind, CoroutineSource, Node};
26-
use rustc_hir::{Expr, HirId};
25+
use rustc_hir::{CoroutineKind, CoroutineSource, Expr, HirId, Node};
2726
use rustc_infer::infer::error_reporting::TypeErrCtxt;
2827
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
2928
use rustc_infer::infer::{BoundRegionConversionTime, DefineOpaqueTypes, InferOk};
@@ -3029,63 +3028,80 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
30293028
err.help("unsized locals are gated as an unstable feature");
30303029
}
30313030
}
3032-
ObligationCauseCode::SizedArgumentType(ty_span) => {
3033-
if let Some(span) = ty_span {
3034-
let trait_len = if let ty::PredicateKind::Clause(clause) =
3035-
predicate.kind().skip_binder()
3036-
&& let ty::ClauseKind::Trait(trait_pred) = clause
3037-
&& let ty::Dynamic(preds, ..) = trait_pred.self_ty().kind()
3038-
{
3039-
let span = if let Ok(snippet) =
3040-
self.tcx.sess.source_map().span_to_snippet(span)
3041-
&& snippet.starts_with("dyn ")
3042-
{
3043-
let pos = snippet.len() - snippet[3..].trim_start().len();
3044-
span.with_hi(span.lo() + BytePos(pos as u32))
3045-
} else {
3046-
span.shrink_to_lo()
3047-
};
3048-
err.span_suggestion_verbose(
3049-
span,
3050-
"you can use `impl Trait` as the argument type",
3051-
"impl ".to_string(),
3052-
Applicability::MaybeIncorrect,
3053-
);
3054-
preds
3055-
.iter()
3056-
.filter(|pred| {
3057-
// We only want to count `dyn Foo + Bar`, not `dyn Foo<Bar>`,
3058-
// because the later doesn't need parentheses.
3059-
matches!(
3060-
pred.skip_binder(),
3061-
ty::ExistentialPredicate::Trait(_)
3062-
| ty::ExistentialPredicate::AutoTrait(_)
3063-
)
3064-
})
3065-
.count()
3066-
} else {
3067-
1
3068-
};
3069-
let sugg = if trait_len == 1 {
3070-
vec![(span.shrink_to_lo(), "&".to_string())]
3071-
} else if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(span)
3072-
&& snippet.starts_with('(')
3073-
{
3074-
// We don't want to suggest `&((dyn Foo + Bar))` when we have
3075-
// `(dyn Foo + Bar)`.
3076-
vec![(span.shrink_to_lo(), "&".to_string())]
3077-
} else {
3078-
vec![
3079-
(span.shrink_to_lo(), "&(".to_string()),
3080-
(span.shrink_to_hi(), ")".to_string()),
3081-
]
3082-
};
3083-
err.multipart_suggestion_verbose(
3084-
"function arguments must have a statically known size, borrowed types \
3085-
always have a known size",
3086-
sugg,
3087-
Applicability::MachineApplicable,
3088-
);
3031+
ObligationCauseCode::SizedArgumentType(hir_id) => {
3032+
let mut ty = None;
3033+
let borrowed_msg = "function arguments must have a statically known size, borrowed \
3034+
types always have a known size";
3035+
if let Some(hir_id) = hir_id
3036+
&& let Some(hir::Node::Param(param)) = self.tcx.hir().find(hir_id)
3037+
&& let Some(item) = self.tcx.hir().find_parent(hir_id)
3038+
&& let Some(decl) = item.fn_decl()
3039+
&& let Some(t) = decl.inputs.iter().find(|t| param.ty_span.contains(t.span))
3040+
{
3041+
// We use `contains` because the type might be surrounded by parentheses,
3042+
// which makes `ty_span` and `t.span` disagree with each other, but one
3043+
// fully contains the other: `foo: (dyn Foo + Bar)`
3044+
// ^-------------^
3045+
// ||
3046+
// |t.span
3047+
// param._ty_span
3048+
ty = Some(t);
3049+
} else if let Some(hir_id) = hir_id
3050+
&& let Some(hir::Node::Ty(t)) = self.tcx.hir().find(hir_id)
3051+
{
3052+
ty = Some(t);
3053+
}
3054+
if let Some(ty) = ty {
3055+
match ty.kind {
3056+
hir::TyKind::TraitObject(traits, _, _) => {
3057+
let (span, kw) = match traits {
3058+
[first, ..] if first.span.lo() == ty.span.lo() => {
3059+
// Missing `dyn` in front of trait object.
3060+
(ty.span.shrink_to_lo(), "dyn ")
3061+
}
3062+
[first, ..] => (ty.span.until(first.span), ""),
3063+
[] => span_bug!(ty.span, "trait object with no traits: {ty:?}"),
3064+
};
3065+
let needs_parens = traits.len() != 1;
3066+
err.span_suggestion_verbose(
3067+
span,
3068+
"you can use `impl Trait` as the argument type",
3069+
"impl ".to_string(),
3070+
Applicability::MaybeIncorrect,
3071+
);
3072+
let sugg = if !needs_parens {
3073+
vec![(span.shrink_to_lo(), format!("&{kw}"))]
3074+
} else {
3075+
vec![
3076+
(span.shrink_to_lo(), format!("&({kw}")),
3077+
(ty.span.shrink_to_hi(), ")".to_string()),
3078+
]
3079+
};
3080+
err.multipart_suggestion_verbose(
3081+
borrowed_msg,
3082+
sugg,
3083+
Applicability::MachineApplicable,
3084+
);
3085+
}
3086+
hir::TyKind::Slice(_ty) => {
3087+
err.span_suggestion_verbose(
3088+
ty.span.shrink_to_lo(),
3089+
"function arguments must have a statically known size, borrowed \
3090+
slices always have a known size",
3091+
"&",
3092+
Applicability::MachineApplicable,
3093+
);
3094+
}
3095+
hir::TyKind::Path(_) => {
3096+
err.span_suggestion_verbose(
3097+
ty.span.shrink_to_lo(),
3098+
borrowed_msg,
3099+
"&",
3100+
Applicability::MachineApplicable,
3101+
);
3102+
}
3103+
_ => {}
3104+
}
30893105
} else {
30903106
err.note("all function arguments must have a statically known size");
30913107
}

‎src/tools/clippy/tests/ui/crashes/ice-6251.stderr

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ LL | fn bug<T>() -> impl Iterator<Item = [(); { |x: [u8]| x }]> {
66
|
77
= help: the trait `std::marker::Sized` is not implemented for `[u8]`
88
= help: unsized fn params are gated as an unstable feature
9-
help: function arguments must have a statically known size, borrowed types always have a known size
9+
help: function arguments must have a statically known size, borrowed slices always have a known size
1010
|
1111
LL | fn bug<T>() -> impl Iterator<Item = [(); { |x: &[u8]| x }]> {
1212
| +

‎tests/ui/feature-gates/feature-gate-unsized_fn_params.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ LL | fn bar(x: impl Foo) {
2929
| ++++
3030
help: function arguments must have a statically known size, borrowed types always have a known size
3131
|
32-
LL | fn bar(x: &Foo) {
33-
| +
32+
LL | fn bar(x: &dyn Foo) {
33+
| ++++
3434

3535
error[E0277]: the size for values of type `[()]` cannot be known at compilation time
3636
--> $DIR/feature-gate-unsized_fn_params.rs:25:8
@@ -40,7 +40,7 @@ LL | fn qux(_: [()]) {}
4040
|
4141
= help: the trait `Sized` is not implemented for `[()]`
4242
= help: unsized fn params are gated as an unstable feature
43-
help: function arguments must have a statically known size, borrowed types always have a known size
43+
help: function arguments must have a statically known size, borrowed slices always have a known size
4444
|
4545
LL | fn qux(_: &[()]) {}
4646
| +

‎tests/ui/resolve/issue-5035-2.stderr

-4
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,6 @@ LL | fn foo(_x: K) {}
66
|
77
= help: the trait `Sized` is not implemented for `(dyn I + 'static)`
88
= help: unsized fn params are gated as an unstable feature
9-
help: you can use `impl Trait` as the argument type
10-
|
11-
LL | fn foo(_x: impl K) {}
12-
| ++++
139
help: function arguments must have a statically known size, borrowed types always have a known size
1410
|
1511
LL | fn foo(_x: &K) {}

‎tests/ui/traits/bound/not-on-bare-trait.stderr

+6-6
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ LL | fn foo(_x: impl Foo + Send) {
3434
| ++++
3535
help: function arguments must have a statically known size, borrowed types always have a known size
3636
|
37-
LL | fn foo(_x: &(Foo + Send)) {
38-
| ++ +
37+
LL | fn foo(_x: &(dyn Foo + Send)) {
38+
| +++++ +
3939

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

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

0 commit comments

Comments
 (0)