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.

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

Provide structured suggestion on unsized fields and fn params #74228

Merged
merged 7 commits into from
Jul 14, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 1 addition & 4 deletions src/libcore/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,8 @@ impl<T: ?Sized> !Send for *mut T {}
#[stable(feature = "rust1", since = "1.0.0")]
#[lang = "sized"]
#[rustc_on_unimplemented(
on(parent_trait = "std::path::Path", label = "borrow the `Path` instead"),
message = "the size for values of type `{Self}` cannot be known at compilation time",
label = "doesn't have a size known at compile-time",
note = "to learn more, visit <https://doc.rust-lang.org/book/\
ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>"
label = "doesn't have a size known at compile-time"
)]
#[fundamental] // for Default, for example, which requires that `[T]: !Default` be evaluatable
#[rustc_specialization_trait]
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_ast_lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
Ident::with_dummy_span(sym::_task_context),
hir::BindingAnnotation::Mutable,
);
let param = hir::Param { attrs: &[], hir_id: self.next_id(), pat, span };
let param = hir::Param { attrs: &[], hir_id: self.next_id(), pat, ty_span: span, span };
let params = arena_vec![self; param];

let body_id = self.lower_body(move |this| {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_ast_lowering/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
attrs: self.lower_attrs(&param.attrs),
hir_id: self.lower_node_id(param.id),
pat: self.lower_pat(&param.pat),
ty_span: param.ty.span,
span: param.span,
}
}
Expand Down Expand Up @@ -1098,6 +1099,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
attrs: parameter.attrs,
hir_id: parameter.hir_id,
pat: new_parameter_pat,
ty_span: parameter.ty_span,
span: parameter.span,
};

Expand Down
1 change: 1 addition & 0 deletions src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2148,6 +2148,7 @@ pub struct Param<'hir> {
pub attrs: &'hir [Attribute],
pub hir_id: HirId,
pub pat: &'hir Pat<'hir>,
pub ty_span: Span,
pub span: Span,
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_middle/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ pub enum ObligationCauseCode<'tcx> {
/// Type of each variable must be `Sized`.
VariableType(hir::HirId),
/// Argument type must be `Sized`.
SizedArgumentType,
SizedArgumentType(Option<Span>),
/// Return type must be `Sized`.
SizedReturnType,
/// Yield type must be `Sized`.
Expand All @@ -229,6 +229,7 @@ pub enum ObligationCauseCode<'tcx> {
/// Types of fields (other than the last, except for packed structs) in a struct must be sized.
FieldSized {
adt_kind: AdtKind,
span: Span,
last: bool,
},

Expand Down
6 changes: 4 additions & 2 deletions src/librustc_middle/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,14 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> {
super::VariableType(id) => Some(super::VariableType(id)),
super::ReturnValue(id) => Some(super::ReturnValue(id)),
super::ReturnType => Some(super::ReturnType),
super::SizedArgumentType => Some(super::SizedArgumentType),
super::SizedArgumentType(sp) => Some(super::SizedArgumentType(sp)),
super::SizedReturnType => Some(super::SizedReturnType),
super::SizedYieldType => Some(super::SizedYieldType),
super::InlineAsmSized => Some(super::InlineAsmSized),
super::RepeatVec(suggest_flag) => Some(super::RepeatVec(suggest_flag)),
super::FieldSized { adt_kind, last } => Some(super::FieldSized { adt_kind, last }),
super::FieldSized { adt_kind, span, last } => {
Some(super::FieldSized { adt_kind, span, last })
}
super::ConstSized => Some(super::ConstSized),
super::ConstPatternStructural => Some(super::ConstPatternStructural),
super::SharedStatic => Some(super::SharedStatic),
Expand Down
8 changes: 6 additions & 2 deletions src/librustc_trait_selection/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,12 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
// If it has a custom `#[rustc_on_unimplemented]`
// error message, let's display it as the label!
err.span_label(span, s.as_str());
err.help(&explanation);
if !matches!(trait_ref.skip_binder().self_ty().kind, ty::Param(_)) {
// When the self type is a type param We don't need to "the trait
// `std::marker::Sized` is not implemented for `T`" as we will point
// at the type param with a label to suggest constraining it.
err.help(&explanation);
}
} else {
err.span_label(span, explanation);
}
Expand All @@ -403,7 +408,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}

self.suggest_dereferences(&obligation, &mut err, &trait_ref, points_at_arg);
self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err);
self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg);
self.suggest_remove_reference(&obligation, &mut err, &trait_ref);
self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref);
Expand Down
137 changes: 82 additions & 55 deletions src/librustc_trait_selection/traits/error_reporting/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,6 @@ pub trait InferCtxtExt<'tcx> {
body_id: hir::HirId,
);

fn suggest_borrow_on_unsized_slice(
&self,
code: &ObligationCauseCode<'tcx>,
err: &mut DiagnosticBuilder<'_>,
);

fn suggest_dereferences(
&self,
obligation: &PredicateObligation<'tcx>,
Expand Down Expand Up @@ -515,32 +509,6 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
}

/// When encountering an assignment of an unsized trait, like `let x = ""[..];`, provide a
/// suggestion to borrow the initializer in order to use have a slice instead.
fn suggest_borrow_on_unsized_slice(
&self,
code: &ObligationCauseCode<'tcx>,
err: &mut DiagnosticBuilder<'_>,
) {
if let &ObligationCauseCode::VariableType(hir_id) = code {
let parent_node = self.tcx.hir().get_parent_node(hir_id);
if let Some(Node::Local(ref local)) = self.tcx.hir().find(parent_node) {
if let Some(ref expr) = local.init {
if let hir::ExprKind::Index(_, _) = expr.kind {
if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(expr.span) {
err.span_suggestion(
expr.span,
"consider borrowing here",
format!("&{}", snippet),
Applicability::MachineApplicable,
);
}
}
}
}
}
}

/// Given a closure's `DefId`, return the given name of the closure.
///
/// This doesn't account for reassignments, but it's only used for suggestions.
Expand Down Expand Up @@ -1817,15 +1785,56 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
}
}
ObligationCauseCode::VariableType(_) => {
err.note("all local variables must have a statically known size");
ObligationCauseCode::VariableType(hir_id) => {
let parent_node = self.tcx.hir().get_parent_node(hir_id);
match self.tcx.hir().find(parent_node) {
Some(Node::Local(hir::Local {
init: Some(hir::Expr { kind: hir::ExprKind::Index(_, _), span, .. }),
..
})) => {
// When encountering an assignment of an unsized trait, like
// `let x = ""[..];`, provide a suggestion to borrow the initializer in
// order to use have a slice instead.
err.span_suggestion_verbose(
span.shrink_to_lo(),
"consider borrowing here",
"&".to_owned(),
Applicability::MachineApplicable,
);
err.note("all local variables must have a statically known size");
}
Some(Node::Param(param)) => {
err.span_suggestion_verbose(
param.ty_span.shrink_to_lo(),
"function arguments must have a statically known size, borrowed types \
always have a known size",
"&".to_owned(),
Applicability::MachineApplicable,
);
}
_ => {
err.note("all local variables must have a statically known size");
}
}
if !self.tcx.features().unsized_locals {
err.help("unsized locals are gated as an unstable feature");
}
}
ObligationCauseCode::SizedArgumentType => {
err.note("all function arguments must have a statically known size");
if !self.tcx.features().unsized_locals {
ObligationCauseCode::SizedArgumentType(sp) => {
if let Some(span) = sp {
err.span_suggestion_verbose(
span.shrink_to_lo(),
"function arguments must have a statically known size, borrowed types \
always have a known size",
"&".to_string(),
Applicability::MachineApplicable,
);
} else {
err.note("all function arguments must have a statically known size");
}
if tcx.sess.opts.unstable_features.is_nightly_build()
&& !self.tcx.features().unsized_locals
{
err.help("unsized locals are gated as an unstable feature");
}
}
Expand All @@ -1844,26 +1853,44 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
ObligationCauseCode::StructInitializerSized => {
err.note("structs must have a statically known size to be initialized");
}
ObligationCauseCode::FieldSized { adt_kind: ref item, last } => match *item {
AdtKind::Struct => {
if last {
err.note(
"the last field of a packed struct may only have a \
dynamically sized type if it does not need drop to be run",
);
} else {
err.note(
"only the last field of a struct may have a dynamically sized type",
);
ObligationCauseCode::FieldSized { adt_kind: ref item, last, span } => {
match *item {
AdtKind::Struct => {
if last {
err.note(
"the last field of a packed struct may only have a \
dynamically sized type if it does not need drop to be run",
);
} else {
err.note(
"only the last field of a struct may have a dynamically sized type",
);
}
}
AdtKind::Union => {
err.note("no field of a union may have a dynamically sized type");
}
AdtKind::Enum => {
err.note("no field of an enum variant may have a dynamically sized type");
}
}
AdtKind::Union => {
err.note("no field of a union may have a dynamically sized type");
}
AdtKind::Enum => {
err.note("no field of an enum variant may have a dynamically sized type");
}
},
err.help("change the field's type to have a statically known size");
err.span_suggestion(
span.shrink_to_lo(),
"borrowed types always have a statically known size",
"&".to_string(),
Applicability::MachineApplicable,
);
err.multipart_suggestion(
"the `Box` type always has a statically known size and allocates its contents \
in the heap",
vec![
(span.shrink_to_lo(), "Box<".to_string()),
(span.shrink_to_hi(), ">".to_string()),
],
Applicability::MachineApplicable,
);
}
ObligationCauseCode::ConstSized => {
err.note("constant expressions must have a statically known size");
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.require_type_is_sized_deferred(
input,
expr.span,
traits::SizedArgumentType,
traits::SizedArgumentType(None),
);
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1342,14 +1342,15 @@ 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.
fcx.check_pat_top(&param.pat, param_ty, try { inputs_hir?.get(idx)?.span }, false);
let ty_span = try { inputs_hir?.get(idx)?.span };
fcx.check_pat_top(&param.pat, param_ty, ty_span, false);

// Check that argument is Sized.
// The check for a non-trivial pattern is a hack to avoid duplicate warnings
// for simple cases like `fn foo(x: Trait)`,
// where we would error once on the parameter as a whole, and once on the binding `x`.
if param.pat.simple_ident().is_none() && !tcx.features().unsized_locals {
fcx.require_type_is_sized(param_ty, param.pat.span, traits::SizedArgumentType);
fcx.require_type_is_sized(param_ty, param.pat.span, traits::SizedArgumentType(ty_span));
}

fcx.write_ty(param.hir_id, param_ty);
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ fn check_type_defn<'tcx, F>(
Some(i) => i,
None => bug!(),
},
span: field.span,
last,
},
),
Expand Down Expand Up @@ -1326,10 +1327,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.iter()
.map(|field| {
let field_ty = self.tcx.type_of(self.tcx.hir().local_def_id(field.hir_id));
let field_ty = self.normalize_associated_types_in(field.span, &field_ty);
let field_ty = self.normalize_associated_types_in(field.ty.span, &field_ty);
let field_ty = self.resolve_vars_if_possible(&field_ty);
debug!("non_enum_variant: type of field {:?} is {:?}", field, field_ty);
AdtField { ty: field_ty, span: field.span }
AdtField { ty: field_ty, span: field.ty.span }
})
.collect();
AdtVariant { fields, explicit_discr: None }
Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/asm/type-check-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ LL | asm!("{}", in(reg) v[..]);
| ^^^^^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `[u64]`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= note: all inline asm arguments must have a statically known size

error[E0277]: the size for values of type `[u64]` cannot be known at compilation time
Expand All @@ -27,7 +26,6 @@ LL | asm!("{}", out(reg) v[..]);
| ^^^^^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `[u64]`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= note: all inline asm arguments must have a statically known size

error[E0277]: the size for values of type `[u64]` cannot be known at compilation time
Expand All @@ -37,7 +35,6 @@ LL | asm!("{}", inout(reg) v[..]);
| ^^^^^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `[u64]`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= note: all inline asm arguments must have a statically known size

error: aborting due to 5 previous errors
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ LL | let x = t.get();
| ^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `<T as Get>::Value`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= note: all local variables must have a statically known size
= help: unsized locals are gated as an unstable feature
help: consider further restricting the associated type
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/associated-types/defaults-suitability.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ LL | pub struct Vec<T> {
| - required by this bound in `std::vec::Vec`
|
= help: the trait `std::marker::Sized` is not implemented for `[u8]`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>

error: aborting due to 11 previous errors

Expand Down
4 changes: 0 additions & 4 deletions src/test/ui/associated-types/defaults-unsound-62211-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ LL | trait UncheckedCopy: Sized {
LL | + AddAssign<&'static str>
| ^^^^^^^^^^^^^^^^^^^^^^^ no implementation for `Self += &'static str`
|
= help: the trait `std::ops::AddAssign<&'static str>` is not implemented for `Self`
help: consider further restricting `Self`
|
LL | trait UncheckedCopy: Sized + std::ops::AddAssign<&'static str> {
Expand Down Expand Up @@ -50,7 +49,6 @@ LL | trait UncheckedCopy: Sized {
LL | + Display = Self;
| ^^^^^^^ `Self` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `Self`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
help: consider further restricting `Self`
|
Expand All @@ -69,7 +67,6 @@ LL | + Display = Self;
LL | impl<T> UncheckedCopy for T {}
| ^^^^^^^^^^^^^ `T` cannot be formatted with the default formatter
|
= help: the trait `std::fmt::Display` is not implemented for `T`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
help: consider restricting type parameter `T`
|
Expand Down Expand Up @@ -105,7 +102,6 @@ LL | + AddAssign<&'static str>
LL | impl<T> UncheckedCopy for T {}
| ^^^^^^^^^^^^^ no implementation for `T += &'static str`
|
= help: the trait `std::ops::AddAssign<&'static str>` is not implemented for `T`
help: consider restricting type parameter `T`
|
LL | impl<T: std::ops::AddAssign<&'static str>> UncheckedCopy for T {}
Expand Down
Loading