From 9dc9785d90f1a54e47679c5ffee8523d0643e3ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 4 Mar 2020 10:50:05 -0800 Subject: [PATCH 01/17] Handle `impl Trait` where `Trait` has an assoc type with missing bounds Fix #69638. --- .../traits/error_reporting/suggestions.rs | 152 +++++++++++++++--- .../impl-trait-with-missing-bounds.rs | 29 ++++ .../impl-trait-with-missing-bounds.stderr | 48 ++++++ 3 files changed, 208 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/suggestions/impl-trait-with-missing-bounds.rs create mode 100644 src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index fcec29aaa8ecb..6606c52c56ff5 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -162,23 +162,123 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { }; let suggest_restriction = - |generics: &hir::Generics<'_>, msg, err: &mut DiagnosticBuilder<'_>| { + |generics: &hir::Generics<'_>, + msg, + err: &mut DiagnosticBuilder<'_>, + fn_sig: Option<&hir::FnSig<'_>>| { + // Type parameter needs more bounds. The trivial case is `T` `where T: Bound`, but + // it can also be an `impl Trait` param that needs to be decomposed to a type + // param for cleaner code. let span = generics.where_clause.span_for_predicates_or_empty_place(); if !span.from_expansion() && span.desugaring_kind().is_none() { - err.span_suggestion( - generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(), - &format!("consider further restricting {}", msg), - format!( - "{} {} ", - if !generics.where_clause.predicates.is_empty() { - "," - } else { - " where" + // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... + if let Some((name, fn_sig)) = fn_sig.and_then(|sig| { + projection.and_then(|p| { + // Shenanigans to get the `Trait` from the `impl Trait`. + match p.self_ty().kind { + ty::Param(param) if param.name.as_str().starts_with("impl ") => { + let n = param.name.as_str(); + // `fn foo(t: impl Trait)` + // ^^^^^ get this string + n.split_whitespace() + .skip(1) + .next() + .map(|n| (n.to_string(), sig)) + } + _ => None, + } + }) + }) { + // FIXME: Cleanup. + let mut ty_spans = vec![]; + let impl_name = format!("impl {}", name); + for i in fn_sig.decl.inputs { + if let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = i.kind { + match path.segments { + [segment] if segment.ident.to_string() == impl_name => { + // `fn foo(t: impl Trait)` + // ^^^^^^^^^^ get this to suggest + // `T` instead + + // There might be more than one `impl Trait`. + ty_spans.push(i.span); + } + _ => {} + } + } + } + + let type_param = format!("{}: {}", "T", name); + // FIXME: modify the `trait_ref` instead of string shenanigans. + // Turn `::Bar: Qux` into `::Bar: Qux`. + let pred = trait_ref.without_const().to_predicate().to_string(); + let pred = pred.replace(&impl_name, "T"); + let mut sugg = vec![ + match generics + .params + .iter() + .filter(|p| match p.kind { + hir::GenericParamKind::Type { + synthetic: Some(hir::SyntheticTyParamKind::ImplTrait), + .. + } => false, + _ => true, + }) + .last() + { + // `fn foo(t: impl Trait)` + // ^ suggest `` here + None => (generics.span, format!("<{}>", type_param)), + Some(param) => { + (param.span.shrink_to_hi(), format!(", {}", type_param)) + } }, - trait_ref.without_const().to_predicate(), - ), - Applicability::MachineApplicable, - ); + ( + // `fn foo(t: impl Trait)` + // ^ suggest `where ::A: Bound` + generics + .where_clause + .span_for_predicates_or_empty_place() + .shrink_to_hi(), + format!( + "{} {} ", + if !generics.where_clause.predicates.is_empty() { + "," + } else { + " where" + }, + pred, + ), + ), + ]; + sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string()))); + // Suggest `fn foo(t: T) where ::A: Bound`. + err.multipart_suggestion( + "introduce a type parameter with a trait bound instead of using \ + `impl Trait`", + sugg, + Applicability::MaybeIncorrect, + ); + } else { + // Trivial case: `T` needs an extra bound. + err.span_suggestion( + generics + .where_clause + .span_for_predicates_or_empty_place() + .shrink_to_hi(), + &format!("consider further restricting {}", msg), + format!( + "{} {} ", + if !generics.where_clause.predicates.is_empty() { + "," + } else { + " where" + }, + trait_ref.without_const().to_predicate(), + ), + Applicability::MachineApplicable, + ); + } } }; @@ -186,6 +286,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // don't suggest `T: Sized + ?Sized`. let mut hir_id = body_id; while let Some(node) = self.tcx.hir().find(hir_id) { + debug!( + "suggest_restricting_param_bound {:?} {:?} {:?} {:?}", + trait_ref, self_ty.kind, projection, node + ); match node { hir::Node::TraitItem(hir::TraitItem { generics, @@ -193,27 +297,33 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { .. }) if param_ty && self_ty == self.tcx.types.self_param => { // Restricting `Self` for a single method. - suggest_restriction(&generics, "`Self`", err); + suggest_restriction(&generics, "`Self`", err, None); return; } hir::Node::TraitItem(hir::TraitItem { generics, - kind: hir::TraitItemKind::Fn(..), + kind: hir::TraitItemKind::Fn(fn_sig, ..), .. }) | hir::Node::ImplItem(hir::ImplItem { generics, - kind: hir::ImplItemKind::Fn(..), + kind: hir::ImplItemKind::Fn(fn_sig, ..), .. }) - | hir::Node::Item( - hir::Item { kind: hir::ItemKind::Fn(_, generics, _), .. } - | hir::Item { kind: hir::ItemKind::Trait(_, _, generics, _, _), .. } + | hir::Node::Item(hir::Item { + kind: hir::ItemKind::Fn(fn_sig, generics, _), .. + }) if projection.is_some() => { + // Missing associated type bound. + suggest_restriction(&generics, "the associated type", err, Some(fn_sig)); + return; + } + hir::Node::Item( + hir::Item { kind: hir::ItemKind::Trait(_, _, generics, _, _), .. } | hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. }, ) if projection.is_some() => { // Missing associated type bound. - suggest_restriction(&generics, "the associated type", err); + suggest_restriction(&generics, "the associated type", err, None); return; } diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs new file mode 100644 index 0000000000000..bef9ba9f91f74 --- /dev/null +++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs @@ -0,0 +1,29 @@ +// The double space in `impl Iterator` is load bearing! We want to make sure we don't regress by +// accident if the internal string representation changes. +#[rustfmt::skip] +fn foo(constraints: impl Iterator) { + for constraint in constraints { + qux(constraint); +//~^ ERROR `::Item` doesn't implement `std::fmt::Debug` + } +} + +fn bar(t: T, constraints: impl Iterator) where T: std::fmt::Debug { + for constraint in constraints { + qux(t); + qux(constraint); +//~^ ERROR `::Item` doesn't implement `std::fmt::Debug` + } +} + +fn baz(t: impl std::fmt::Debug, constraints: impl Iterator) { + for constraint in constraints { + qux(t); + qux(constraint); +//~^ ERROR `::Item` doesn't implement `std::fmt::Debug` + } +} + +fn qux(_: impl std::fmt::Debug) {} + +fn main() {} diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr new file mode 100644 index 0000000000000..5f84e6d44ea9d --- /dev/null +++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr @@ -0,0 +1,48 @@ +error[E0277]: `::Item` doesn't implement `std::fmt::Debug` + --> $DIR/impl-trait-with-missing-bounds.rs:6:13 + | +LL | qux(constraint); + | ^^^^^^^^^^ `::Item` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` +... +LL | fn qux(_: impl std::fmt::Debug) {} + | --- --------------- required by this bound in `qux` + | + = help: the trait `std::fmt::Debug` is not implemented for `::Item` +help: introduce a type parameter with a trait bound instead of using `impl Trait` + | +LL | fn foo(constraints: T) where ::Item: std::fmt::Debug { + | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0277]: `::Item` doesn't implement `std::fmt::Debug` + --> $DIR/impl-trait-with-missing-bounds.rs:14:13 + | +LL | qux(constraint); + | ^^^^^^^^^^ `::Item` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` +... +LL | fn qux(_: impl std::fmt::Debug) {} + | --- --------------- required by this bound in `qux` + | + = help: the trait `std::fmt::Debug` is not implemented for `::Item` +help: introduce a type parameter with a trait bound instead of using `impl Trait` + | +LL | fn bar(t: T, constraints: T) where T: std::fmt::Debug, ::Item: std::fmt::Debug { + | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0277]: `::Item` doesn't implement `std::fmt::Debug` + --> $DIR/impl-trait-with-missing-bounds.rs:22:13 + | +LL | qux(constraint); + | ^^^^^^^^^^ `::Item` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` +... +LL | fn qux(_: impl std::fmt::Debug) {} + | --- --------------- required by this bound in `qux` + | + = help: the trait `std::fmt::Debug` is not implemented for `::Item` +help: introduce a type parameter with a trait bound instead of using `impl Trait` + | +LL | fn baz(t: impl std::fmt::Debug, constraints: T) where ::Item: std::fmt::Debug { + | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0277`. From 02711fdeb685c5c27f02b984290759abe900b3b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 4 Apr 2020 17:31:32 -0700 Subject: [PATCH 02/17] review comments --- src/librustc_trait_selection/lib.rs | 1 + .../traits/error_reporting/suggestions.rs | 269 +++++++++--------- 2 files changed, 140 insertions(+), 130 deletions(-) diff --git a/src/librustc_trait_selection/lib.rs b/src/librustc_trait_selection/lib.rs index 21315cc89ca5c..fb82a50cd16ef 100644 --- a/src/librustc_trait_selection/lib.rs +++ b/src/librustc_trait_selection/lib.rs @@ -16,6 +16,7 @@ #![feature(in_band_lifetimes)] #![feature(crate_visibility_modifier)] #![feature(or_patterns)] +#![feature(str_strip)] #![recursion_limit = "512"] // For rustdoc #[macro_use] diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 6606c52c56ff5..55daf936425d4 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -147,6 +147,126 @@ crate trait InferCtxtExt<'tcx> { fn suggest_new_overflow_limit(&self, err: &mut DiagnosticBuilder<'_>); } +fn predicate_constraint(generics: &hir::Generics<'_>, pred: String) -> (Span, String) { + ( + generics.where_clause.span_for_predicates_or_empty_place().shrink_to_hi(), + format!( + "{} {} ", + if !generics.where_clause.predicates.is_empty() { "," } else { " where" }, + pred, + ), + ) +} + +/// Type parameter needs more bounds. The trivial case is `T` `where T: Bound`, but +/// it can also be an `impl Trait` param that needs to be decomposed to a type +/// param for cleaner code. +fn suggest_restriction( + generics: &hir::Generics<'_>, + msg: &str, + err: &mut DiagnosticBuilder<'_>, + fn_sig: Option<&hir::FnSig<'_>>, + projection: Option<&ty::ProjectionTy<'_>>, + trait_ref: &ty::PolyTraitRef<'_>, +) { + let span = generics.where_clause.span_for_predicates_or_empty_place(); + if !span.from_expansion() && span.desugaring_kind().is_none() { + // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... + if let Some((name, fn_sig)) = fn_sig.and_then(|sig| { + projection.and_then(|p| { + // Shenanigans to get the `Trait` from the `impl Trait`. + match p.self_ty().kind { + ty::Param(param) => { + // `fn foo(t: impl Trait)` + // ^^^^^ get this string + param + .name + .as_str() + .strip_prefix("impl") + .map(|s| (s.trim_start().to_string(), sig)) + } + _ => None, + } + }) + }) { + // We know we have an `impl Trait` that doesn't satisfy a required projection. + + // Find all of the ocurrences of `impl Trait` for `Trait` in the function arguments' + // types. There should be at least one, but there might be *more* than one. In that + // case we could just ignore it and try to identify which one needs the restriction, + // but instead we choose to suggest replacing all instances of `impl Trait` with `T` + // where `T: Trait`. + let mut ty_spans = vec![]; + let impl_name = format!("impl {}", name); + for input in fn_sig.decl.inputs { + if let hir::TyKind::Path(hir::QPath::Resolved( + None, + hir::Path { segments: [segment], .. }, + )) = input.kind + { + if segment.ident.as_str() == impl_name.as_str() { + // `fn foo(t: impl Trait)` + // ^^^^^^^^^^ get this to suggest + // `T` instead + + // There might be more than one `impl Trait`. + ty_spans.push(input.span); + } + } + } + + // The type param `T: Trait` we will suggest to introduce. + let type_param = format!("{}: {}", "T", name); + + // FIXME: modify the `trait_ref` instead of string shenanigans. + // Turn `::Bar: Qux` into `::Bar: Qux`. + let pred = trait_ref.without_const().to_predicate().to_string(); + let pred = pred.replace(&impl_name, "T"); + let mut sugg = vec![ + match generics + .params + .iter() + .filter(|p| match p.kind { + hir::GenericParamKind::Type { + synthetic: Some(hir::SyntheticTyParamKind::ImplTrait), + .. + } => false, + _ => true, + }) + .last() + { + // `fn foo(t: impl Trait)` + // ^ suggest `` here + None => (generics.span, format!("<{}>", type_param)), + // `fn foo(t: impl Trait)` + // ^^^ suggest `` here + Some(param) => (param.span.shrink_to_hi(), format!(", {}", type_param)), + }, + // `fn foo(t: impl Trait)` + // ^ suggest `where ::A: Bound` + predicate_constraint(generics, pred), + ]; + sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string()))); + + // Suggest `fn foo(t: T) where ::A: Bound`. + err.multipart_suggestion( + "introduce a type parameter with a trait bound instead of using \ + `impl Trait`", + sugg, + Applicability::MaybeIncorrect, + ); + } else { + // Trivial case: `T` needs an extra bound: `T: Bound`. + let (sp, s) = predicate_constraint( + generics, + trait_ref.without_const().to_predicate().to_string(), + ); + let appl = Applicability::MachineApplicable; + err.span_suggestion(sp, &format!("consider further restricting {}", msg), s, appl); + } + } +} + impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { fn suggest_restricting_param_bound( &self, @@ -161,135 +281,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { _ => return, }; - let suggest_restriction = - |generics: &hir::Generics<'_>, - msg, - err: &mut DiagnosticBuilder<'_>, - fn_sig: Option<&hir::FnSig<'_>>| { - // Type parameter needs more bounds. The trivial case is `T` `where T: Bound`, but - // it can also be an `impl Trait` param that needs to be decomposed to a type - // param for cleaner code. - let span = generics.where_clause.span_for_predicates_or_empty_place(); - if !span.from_expansion() && span.desugaring_kind().is_none() { - // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... - if let Some((name, fn_sig)) = fn_sig.and_then(|sig| { - projection.and_then(|p| { - // Shenanigans to get the `Trait` from the `impl Trait`. - match p.self_ty().kind { - ty::Param(param) if param.name.as_str().starts_with("impl ") => { - let n = param.name.as_str(); - // `fn foo(t: impl Trait)` - // ^^^^^ get this string - n.split_whitespace() - .skip(1) - .next() - .map(|n| (n.to_string(), sig)) - } - _ => None, - } - }) - }) { - // FIXME: Cleanup. - let mut ty_spans = vec![]; - let impl_name = format!("impl {}", name); - for i in fn_sig.decl.inputs { - if let hir::TyKind::Path(hir::QPath::Resolved(None, path)) = i.kind { - match path.segments { - [segment] if segment.ident.to_string() == impl_name => { - // `fn foo(t: impl Trait)` - // ^^^^^^^^^^ get this to suggest - // `T` instead - - // There might be more than one `impl Trait`. - ty_spans.push(i.span); - } - _ => {} - } - } - } - - let type_param = format!("{}: {}", "T", name); - // FIXME: modify the `trait_ref` instead of string shenanigans. - // Turn `::Bar: Qux` into `::Bar: Qux`. - let pred = trait_ref.without_const().to_predicate().to_string(); - let pred = pred.replace(&impl_name, "T"); - let mut sugg = vec![ - match generics - .params - .iter() - .filter(|p| match p.kind { - hir::GenericParamKind::Type { - synthetic: Some(hir::SyntheticTyParamKind::ImplTrait), - .. - } => false, - _ => true, - }) - .last() - { - // `fn foo(t: impl Trait)` - // ^ suggest `` here - None => (generics.span, format!("<{}>", type_param)), - Some(param) => { - (param.span.shrink_to_hi(), format!(", {}", type_param)) - } - }, - ( - // `fn foo(t: impl Trait)` - // ^ suggest `where ::A: Bound` - generics - .where_clause - .span_for_predicates_or_empty_place() - .shrink_to_hi(), - format!( - "{} {} ", - if !generics.where_clause.predicates.is_empty() { - "," - } else { - " where" - }, - pred, - ), - ), - ]; - sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string()))); - // Suggest `fn foo(t: T) where ::A: Bound`. - err.multipart_suggestion( - "introduce a type parameter with a trait bound instead of using \ - `impl Trait`", - sugg, - Applicability::MaybeIncorrect, - ); - } else { - // Trivial case: `T` needs an extra bound. - err.span_suggestion( - generics - .where_clause - .span_for_predicates_or_empty_place() - .shrink_to_hi(), - &format!("consider further restricting {}", msg), - format!( - "{} {} ", - if !generics.where_clause.predicates.is_empty() { - "," - } else { - " where" - }, - trait_ref.without_const().to_predicate(), - ), - Applicability::MachineApplicable, - ); - } - } - }; - // FIXME: Add check for trait bound that is already present, particularly `?Sized` so we // don't suggest `T: Sized + ?Sized`. let mut hir_id = body_id; while let Some(node) = self.tcx.hir().find(hir_id) { - debug!( - "suggest_restricting_param_bound {:?} {:?} {:?} {:?}", - trait_ref, self_ty.kind, projection, node - ); match node { hir::Node::TraitItem(hir::TraitItem { generics, @@ -297,7 +292,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { .. }) if param_ty && self_ty == self.tcx.types.self_param => { // Restricting `Self` for a single method. - suggest_restriction(&generics, "`Self`", err, None); + suggest_restriction(&generics, "`Self`", err, None, projection, trait_ref); return; } @@ -314,16 +309,30 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { | hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(fn_sig, generics, _), .. }) if projection.is_some() => { - // Missing associated type bound. - suggest_restriction(&generics, "the associated type", err, Some(fn_sig)); + // Missing restriction on associated type of type parameter (unmet projection). + suggest_restriction( + &generics, + "the associated type", + err, + Some(fn_sig), + projection, + trait_ref, + ); return; } hir::Node::Item( hir::Item { kind: hir::ItemKind::Trait(_, _, generics, _, _), .. } | hir::Item { kind: hir::ItemKind::Impl { generics, .. }, .. }, ) if projection.is_some() => { - // Missing associated type bound. - suggest_restriction(&generics, "the associated type", err, None); + // Missing restriction on associated type of type parameter (unmet projection). + suggest_restriction( + &generics, + "the associated type", + err, + None, + projection, + trait_ref, + ); return; } From 02ccf861b98fcf23e208acfc0b0fc6f83502d3b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 5 Apr 2020 10:52:54 -0700 Subject: [PATCH 03/17] review comments --- src/librustc_trait_selection/lib.rs | 1 + .../traits/error_reporting/mod.rs | 2 +- .../traits/error_reporting/suggestions.rs | 175 +++++++++--------- 3 files changed, 86 insertions(+), 92 deletions(-) diff --git a/src/librustc_trait_selection/lib.rs b/src/librustc_trait_selection/lib.rs index fb82a50cd16ef..9ada88098a5b5 100644 --- a/src/librustc_trait_selection/lib.rs +++ b/src/librustc_trait_selection/lib.rs @@ -17,6 +17,7 @@ #![feature(crate_visibility_modifier)] #![feature(or_patterns)] #![feature(str_strip)] +#![feature(option_zip)] #![recursion_limit = "512"] // For rustdoc #[macro_use] diff --git a/src/librustc_trait_selection/traits/error_reporting/mod.rs b/src/librustc_trait_selection/traits/error_reporting/mod.rs index 5a9a96887f66a..44787644d027b 100644 --- a/src/librustc_trait_selection/traits/error_reporting/mod.rs +++ b/src/librustc_trait_selection/traits/error_reporting/mod.rs @@ -387,7 +387,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { // which is somewhat confusing. self.suggest_restricting_param_bound( &mut err, - &trait_ref, + trait_ref, obligation.cause.body_id, ); } else { diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 55daf936425d4..e9a9ba921b817 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -26,7 +26,7 @@ crate trait InferCtxtExt<'tcx> { fn suggest_restricting_param_bound( &self, err: &mut DiagnosticBuilder<'_>, - trait_ref: &ty::PolyTraitRef<'_>, + trait_ref: ty::PolyTraitRef<'_>, body_id: hir::HirId, ); @@ -167,103 +167,96 @@ fn suggest_restriction( err: &mut DiagnosticBuilder<'_>, fn_sig: Option<&hir::FnSig<'_>>, projection: Option<&ty::ProjectionTy<'_>>, - trait_ref: &ty::PolyTraitRef<'_>, + trait_ref: ty::PolyTraitRef<'_>, ) { let span = generics.where_clause.span_for_predicates_or_empty_place(); - if !span.from_expansion() && span.desugaring_kind().is_none() { - // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... - if let Some((name, fn_sig)) = fn_sig.and_then(|sig| { - projection.and_then(|p| { - // Shenanigans to get the `Trait` from the `impl Trait`. - match p.self_ty().kind { - ty::Param(param) => { - // `fn foo(t: impl Trait)` - // ^^^^^ get this string - param - .name - .as_str() - .strip_prefix("impl") - .map(|s| (s.trim_start().to_string(), sig)) - } - _ => None, - } - }) - }) { - // We know we have an `impl Trait` that doesn't satisfy a required projection. - - // Find all of the ocurrences of `impl Trait` for `Trait` in the function arguments' - // types. There should be at least one, but there might be *more* than one. In that - // case we could just ignore it and try to identify which one needs the restriction, - // but instead we choose to suggest replacing all instances of `impl Trait` with `T` - // where `T: Trait`. - let mut ty_spans = vec![]; - let impl_name = format!("impl {}", name); - for input in fn_sig.decl.inputs { - if let hir::TyKind::Path(hir::QPath::Resolved( - None, - hir::Path { segments: [segment], .. }, - )) = input.kind - { - if segment.ident.as_str() == impl_name.as_str() { - // `fn foo(t: impl Trait)` - // ^^^^^^^^^^ get this to suggest - // `T` instead + if span.from_expansion() || span.desugaring_kind().is_some() { + return; + } + // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... + if let Some((name, fn_sig)) = + fn_sig.zip(projection).and_then(|(sig, p)| match p.self_ty().kind { + // Shenanigans to get the `Trait` from the `impl Trait`. + ty::Param(param) => { + // `fn foo(t: impl Trait)` + // ^^^^^ get this string + param.name.as_str().strip_prefix("impl").map(|s| (s.trim_start().to_string(), sig)) + } + _ => None, + }) + { + // We know we have an `impl Trait` that doesn't satisfy a required projection. + + // Find all of the ocurrences of `impl Trait` for `Trait` in the function arguments' + // types. There should be at least one, but there might be *more* than one. In that + // case we could just ignore it and try to identify which one needs the restriction, + // but instead we choose to suggest replacing all instances of `impl Trait` with `T` + // where `T: Trait`. + let mut ty_spans = vec![]; + let impl_name = format!("impl {}", name); + for input in fn_sig.decl.inputs { + if let hir::TyKind::Path(hir::QPath::Resolved( + None, + hir::Path { segments: [segment], .. }, + )) = input.kind + { + if segment.ident.as_str() == impl_name.as_str() { + // `fn foo(t: impl Trait)` + // ^^^^^^^^^^ get this to suggest + // `T` instead - // There might be more than one `impl Trait`. - ty_spans.push(input.span); - } + // There might be more than one `impl Trait`. + ty_spans.push(input.span); } } + } - // The type param `T: Trait` we will suggest to introduce. - let type_param = format!("{}: {}", "T", name); - - // FIXME: modify the `trait_ref` instead of string shenanigans. - // Turn `::Bar: Qux` into `::Bar: Qux`. - let pred = trait_ref.without_const().to_predicate().to_string(); - let pred = pred.replace(&impl_name, "T"); - let mut sugg = vec![ - match generics - .params - .iter() - .filter(|p| match p.kind { - hir::GenericParamKind::Type { - synthetic: Some(hir::SyntheticTyParamKind::ImplTrait), - .. - } => false, - _ => true, - }) - .last() - { - // `fn foo(t: impl Trait)` - // ^ suggest `` here - None => (generics.span, format!("<{}>", type_param)), - // `fn foo(t: impl Trait)` - // ^^^ suggest `` here - Some(param) => (param.span.shrink_to_hi(), format!(", {}", type_param)), - }, + // The type param `T: Trait` we will suggest to introduce. + let type_param = format!("{}: {}", "T", name); + + // FIXME: modify the `trait_ref` instead of string shenanigans. + // Turn `::Bar: Qux` into `::Bar: Qux`. + let pred = trait_ref.without_const().to_predicate().to_string(); + let pred = pred.replace(&impl_name, "T"); + let mut sugg = vec![ + match generics + .params + .iter() + .filter(|p| match p.kind { + hir::GenericParamKind::Type { + synthetic: Some(hir::SyntheticTyParamKind::ImplTrait), + .. + } => false, + _ => true, + }) + .last() + { // `fn foo(t: impl Trait)` - // ^ suggest `where ::A: Bound` - predicate_constraint(generics, pred), - ]; - sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string()))); - - // Suggest `fn foo(t: T) where ::A: Bound`. - err.multipart_suggestion( - "introduce a type parameter with a trait bound instead of using \ + // ^ suggest `` here + None => (generics.span, format!("<{}>", type_param)), + // `fn foo(t: impl Trait)` + // ^^^ suggest `` here + Some(param) => (param.span.shrink_to_hi(), format!(", {}", type_param)), + }, + // `fn foo(t: impl Trait)` + // ^ suggest `where ::A: Bound` + predicate_constraint(generics, pred), + ]; + sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string()))); + + // Suggest `fn foo(t: T) where ::A: Bound`. + err.multipart_suggestion( + "introduce a type parameter with a trait bound instead of using \ `impl Trait`", - sugg, - Applicability::MaybeIncorrect, - ); - } else { - // Trivial case: `T` needs an extra bound: `T: Bound`. - let (sp, s) = predicate_constraint( - generics, - trait_ref.without_const().to_predicate().to_string(), - ); - let appl = Applicability::MachineApplicable; - err.span_suggestion(sp, &format!("consider further restricting {}", msg), s, appl); - } + sugg, + Applicability::MaybeIncorrect, + ); + } else { + // Trivial case: `T` needs an extra bound: `T: Bound`. + let (sp, s) = + predicate_constraint(generics, trait_ref.without_const().to_predicate().to_string()); + let appl = Applicability::MachineApplicable; + err.span_suggestion(sp, &format!("consider further restricting {}", msg), s, appl); } } @@ -271,7 +264,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> { fn suggest_restricting_param_bound( &self, mut err: &mut DiagnosticBuilder<'_>, - trait_ref: &ty::PolyTraitRef<'_>, + trait_ref: ty::PolyTraitRef<'_>, body_id: hir::HirId, ) { let self_ty = trait_ref.self_ty(); From f26c8559b837b892681f24fde9d909bb5dc4c082 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 5 Apr 2020 12:32:34 -0700 Subject: [PATCH 04/17] Account for type params with bounds --- .../traits/error_reporting/suggestions.rs | 11 ++++++----- .../impl-trait-with-missing-bounds.rs | 8 ++++++++ .../impl-trait-with-missing-bounds.stderr | 17 ++++++++++++++++- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index e9a9ba921b817..ec131cd593765 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -202,8 +202,7 @@ fn suggest_restriction( { if segment.ident.as_str() == impl_name.as_str() { // `fn foo(t: impl Trait)` - // ^^^^^^^^^^ get this to suggest - // `T` instead + // ^^^^^^^^^^ get this to suggest `T` instead // There might be more than one `impl Trait`. ty_spans.push(input.span); @@ -236,7 +235,10 @@ fn suggest_restriction( None => (generics.span, format!("<{}>", type_param)), // `fn foo(t: impl Trait)` // ^^^ suggest `` here - Some(param) => (param.span.shrink_to_hi(), format!(", {}", type_param)), + Some(param) => ( + param.bounds_span().unwrap_or(param.span).shrink_to_hi(), + format!(", {}", type_param), + ), }, // `fn foo(t: impl Trait)` // ^ suggest `where ::A: Bound` @@ -246,8 +248,7 @@ fn suggest_restriction( // Suggest `fn foo(t: T) where ::A: Bound`. err.multipart_suggestion( - "introduce a type parameter with a trait bound instead of using \ - `impl Trait`", + "introduce a type parameter with a trait bound instead of using `impl Trait`", sugg, Applicability::MaybeIncorrect, ); diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs index bef9ba9f91f74..d831bafa2b51c 100644 --- a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs +++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs @@ -24,6 +24,14 @@ fn baz(t: impl std::fmt::Debug, constraints: impl Iterator) { } } +fn bat(t: T, constraints: impl Iterator) { + for constraint in constraints { + qux(t); + qux(constraint); +//~^ ERROR `::Item` doesn't implement `std::fmt::Debug` + } +} + fn qux(_: impl std::fmt::Debug) {} fn main() {} diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr index 5f84e6d44ea9d..f0f47876ed0f2 100644 --- a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr +++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr @@ -43,6 +43,21 @@ help: introduce a type parameter with a trait bound instead of using `impl Trait LL | fn baz(t: impl std::fmt::Debug, constraints: T) where ::Item: std::fmt::Debug { | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 3 previous errors +error[E0277]: `::Item` doesn't implement `std::fmt::Debug` + --> $DIR/impl-trait-with-missing-bounds.rs:30:13 + | +LL | qux(constraint); + | ^^^^^^^^^^ `::Item` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` +... +LL | fn qux(_: impl std::fmt::Debug) {} + | --- --------------- required by this bound in `qux` + | + = help: the trait `std::fmt::Debug` is not implemented for `::Item` +help: introduce a type parameter with a trait bound instead of using `impl Trait` + | +LL | fn bat(t: T, constraints: T) where ::Item: std::fmt::Debug { + | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 4 previous errors For more information about this error, try `rustc --explain E0277`. From ba755a8f9e98b6c8c28ffdfc494fc9ae1d72e79b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 5 Apr 2020 13:13:13 -0700 Subject: [PATCH 05/17] Account for existing names when suggesting adding a type param --- src/librustc_hir/hir.rs | 23 +++++++++++++++++++ .../traits/error_reporting/suggestions.rs | 9 ++++---- src/librustc_typeck/collect.rs | 17 ++------------ .../impl-trait-with-missing-bounds.rs | 9 +++++++- .../impl-trait-with-missing-bounds.stderr | 23 +++++++++++++++---- .../typeck_type_placeholder_item.stderr | 6 ++--- 6 files changed, 60 insertions(+), 27 deletions(-) diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index b719d576d6f67..b4744a7d6db1f 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -437,6 +437,29 @@ impl GenericParam<'hir> { } } +pub trait NextTypeParamName { + fn next_type_param_name(&self) -> &'static str; +} + +impl NextTypeParamName for &[GenericParam<'_>] { + fn next_type_param_name(&self) -> &'static str { + // This is the whitelist of possible parameter names that we might suggest. + let possible_names = ["T", "U", "V", "X", "Y", "Z", "A", "B", "C", "D", "E", "F", "G"]; + let used_names = self + .iter() + .filter_map(|p| match p.name { + ParamName::Plain(ident) => Some(ident.name), + _ => None, + }) + .collect::>(); + + possible_names + .iter() + .find(|n| !used_names.contains(&Symbol::intern(n))) + .unwrap_or(&"ParamName") + } +} + #[derive(Default)] pub struct GenericParamCount { pub lifetimes: usize, diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index ec131cd593765..6758301151bd5 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -10,7 +10,7 @@ use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; -use rustc_hir::Node; +use rustc_hir::{NextTypeParamName, Node}; use rustc_middle::ty::TypeckTables; use rustc_middle::ty::{ self, AdtKind, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness, @@ -210,13 +210,14 @@ fn suggest_restriction( } } + let type_param_name = generics.params.next_type_param_name(); // The type param `T: Trait` we will suggest to introduce. - let type_param = format!("{}: {}", "T", name); + let type_param = format!("{}: {}", type_param_name, name); // FIXME: modify the `trait_ref` instead of string shenanigans. // Turn `::Bar: Qux` into `::Bar: Qux`. let pred = trait_ref.without_const().to_predicate().to_string(); - let pred = pred.replace(&impl_name, "T"); + let pred = pred.replace(&impl_name, type_param_name); let mut sugg = vec![ match generics .params @@ -244,7 +245,7 @@ fn suggest_restriction( // ^ suggest `where ::A: Bound` predicate_constraint(generics, pred), ]; - sugg.extend(ty_spans.into_iter().map(|s| (s, "T".to_string()))); + sugg.extend(ty_spans.into_iter().map(|s| (s, type_param_name.to_string()))); // Suggest `fn foo(t: T) where ::A: Bound`. err.multipart_suggestion( diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 13c6670f6b226..3839cfb37abc5 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -29,7 +29,7 @@ use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::weak_lang_items; -use rustc_hir::{GenericParamKind, Node, Unsafety}; +use rustc_hir::{GenericParamKind, NextTypeParamName, Node, Unsafety}; use rustc_middle::hir::map::blocks::FnLikeNode; use rustc_middle::hir::map::Map; use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; @@ -135,20 +135,7 @@ crate fn placeholder_type_error( if placeholder_types.is_empty() { return; } - // This is the whitelist of possible parameter names that we might suggest. - let possible_names = ["T", "K", "L", "A", "B", "C"]; - let used_names = generics - .iter() - .filter_map(|p| match p.name { - hir::ParamName::Plain(ident) => Some(ident.name), - _ => None, - }) - .collect::>(); - - let type_name = possible_names - .iter() - .find(|n| !used_names.contains(&Symbol::intern(n))) - .unwrap_or(&"ParamName"); + let type_name = generics.next_type_param_name(); let mut sugg: Vec<_> = placeholder_types.iter().map(|sp| (*sp, (*type_name).to_string())).collect(); diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs index d831bafa2b51c..6947bc0a734b9 100644 --- a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs +++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs @@ -24,7 +24,7 @@ fn baz(t: impl std::fmt::Debug, constraints: impl Iterator) { } } -fn bat(t: T, constraints: impl Iterator) { +fn bat(t: T, constraints: impl Iterator, _: K) { for constraint in constraints { qux(t); qux(constraint); @@ -32,6 +32,13 @@ fn bat(t: T, constraints: impl Iterator) { } } +fn bak(constraints: impl Iterator + std::fmt::Debug) { + for constraint in constraints { + qux(constraint); +//~^ ERROR `::Item` doesn't implement + } +} + fn qux(_: impl std::fmt::Debug) {} fn main() {} diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr index f0f47876ed0f2..2d48be42233ea 100644 --- a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr +++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr @@ -25,7 +25,7 @@ LL | fn qux(_: impl std::fmt::Debug) {} = help: the trait `std::fmt::Debug` is not implemented for `::Item` help: introduce a type parameter with a trait bound instead of using `impl Trait` | -LL | fn bar(t: T, constraints: T) where T: std::fmt::Debug, ::Item: std::fmt::Debug { +LL | fn bar(t: T, constraints: U) where T: std::fmt::Debug, ::Item: std::fmt::Debug { | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0277]: `::Item` doesn't implement `std::fmt::Debug` @@ -55,9 +55,24 @@ LL | fn qux(_: impl std::fmt::Debug) {} = help: the trait `std::fmt::Debug` is not implemented for `::Item` help: introduce a type parameter with a trait bound instead of using `impl Trait` | -LL | fn bat(t: T, constraints: T) where ::Item: std::fmt::Debug { - | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | fn bat(t: T, constraints: U, _: K) where ::Item: std::fmt::Debug { + | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 4 previous errors +error[E0277]: `::Item` doesn't implement `std::fmt::Debug` + --> $DIR/impl-trait-with-missing-bounds.rs:37:13 + | +LL | qux(constraint); + | ^^^^^^^^^^ `::Item` cannot be formatted using `{:?}` because it doesn't implement `std::fmt::Debug` +... +LL | fn qux(_: impl std::fmt::Debug) {} + | --- --------------- required by this bound in `qux` + | + = help: the trait `std::fmt::Debug` is not implemented for `::Item` +help: introduce a type parameter with a trait bound instead of using `impl Trait` + | +LL | fn bak(constraints: T) where ::Item: std::fmt::Debug { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 5 previous errors For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/typeck/typeck_type_placeholder_item.stderr b/src/test/ui/typeck/typeck_type_placeholder_item.stderr index db67e0c9b7d98..6c0653d5fcb7c 100644 --- a/src/test/ui/typeck/typeck_type_placeholder_item.stderr +++ b/src/test/ui/typeck/typeck_type_placeholder_item.stderr @@ -106,7 +106,7 @@ LL | fn test6_b(_: _, _: T) { } | help: use type parameters instead | -LL | fn test6_b(_: K, _: T) { } +LL | fn test6_b(_: U, _: T) { } | ^^^ ^ error[E0121]: the type placeholder `_` is not allowed within types on item signatures @@ -117,7 +117,7 @@ LL | fn test6_c(_: _, _: (T, K, L, A, B)) { } | help: use type parameters instead | -LL | fn test6_c(_: C, _: (T, K, L, A, B)) { } +LL | fn test6_c(_: U, _: (T, K, L, A, B)) { } | ^^^ ^ error[E0121]: the type placeholder `_` is not allowed within types on item signatures @@ -377,7 +377,7 @@ LL | struct BadStruct2<_, T>(_, T); | help: use type parameters instead | -LL | struct BadStruct2(K, T); +LL | struct BadStruct2(U, T); | ^ ^ error[E0121]: the type placeholder `_` is not allowed within types on item signatures From 5e07be49a28d7314c8b80ffe44df33d322c5dd53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 5 Apr 2020 14:55:06 -0700 Subject: [PATCH 06/17] Try to use the first char in the trait name as type param --- src/librustc_hir/hir.rs | 9 ++++++--- .../traits/error_reporting/suggestions.rs | 4 ++-- src/librustc_typeck/collect.rs | 2 +- .../ui/suggestions/impl-trait-with-missing-bounds.rs | 2 +- .../suggestions/impl-trait-with-missing-bounds.stderr | 10 +++++----- 5 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index b4744a7d6db1f..f26fc402a9ac2 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -438,13 +438,15 @@ impl GenericParam<'hir> { } pub trait NextTypeParamName { - fn next_type_param_name(&self) -> &'static str; + fn next_type_param_name(&self, name: Option<&str>) -> String; } impl NextTypeParamName for &[GenericParam<'_>] { - fn next_type_param_name(&self) -> &'static str { + fn next_type_param_name(&self, name: Option<&str>) -> String { // This is the whitelist of possible parameter names that we might suggest. - let possible_names = ["T", "U", "V", "X", "Y", "Z", "A", "B", "C", "D", "E", "F", "G"]; + let name = name.and_then(|n| n.chars().next()).map(|c| c.to_string().to_uppercase()); + let name = name.as_ref().map(|s| s.as_str()); + let possible_names = [name.unwrap_or("T"), "T", "U", "V", "X", "Y", "Z", "A", "B", "C"]; let used_names = self .iter() .filter_map(|p| match p.name { @@ -457,6 +459,7 @@ impl NextTypeParamName for &[GenericParam<'_>] { .iter() .find(|n| !used_names.contains(&Symbol::intern(n))) .unwrap_or(&"ParamName") + .to_string() } } diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 6758301151bd5..46cb297ff1353 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -210,14 +210,14 @@ fn suggest_restriction( } } - let type_param_name = generics.params.next_type_param_name(); + let type_param_name = generics.params.next_type_param_name(Some(&name)); // The type param `T: Trait` we will suggest to introduce. let type_param = format!("{}: {}", type_param_name, name); // FIXME: modify the `trait_ref` instead of string shenanigans. // Turn `::Bar: Qux` into `::Bar: Qux`. let pred = trait_ref.without_const().to_predicate().to_string(); - let pred = pred.replace(&impl_name, type_param_name); + let pred = pred.replace(&impl_name, &type_param_name); let mut sugg = vec![ match generics .params diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 3839cfb37abc5..10fb7c32e9217 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -135,7 +135,7 @@ crate fn placeholder_type_error( if placeholder_types.is_empty() { return; } - let type_name = generics.next_type_param_name(); + let type_name = generics.next_type_param_name(None); let mut sugg: Vec<_> = placeholder_types.iter().map(|sp| (*sp, (*type_name).to_string())).collect(); diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs index 6947bc0a734b9..6e9e8821cfea7 100644 --- a/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs +++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.rs @@ -24,7 +24,7 @@ fn baz(t: impl std::fmt::Debug, constraints: impl Iterator) { } } -fn bat(t: T, constraints: impl Iterator, _: K) { +fn bat(t: T, constraints: impl Iterator, _: I) { for constraint in constraints { qux(t); qux(constraint); diff --git a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr index 2d48be42233ea..e1c40e2537b3e 100644 --- a/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr +++ b/src/test/ui/suggestions/impl-trait-with-missing-bounds.stderr @@ -10,7 +10,7 @@ LL | fn qux(_: impl std::fmt::Debug) {} = help: the trait `std::fmt::Debug` is not implemented for `::Item` help: introduce a type parameter with a trait bound instead of using `impl Trait` | -LL | fn foo(constraints: T) where ::Item: std::fmt::Debug { +LL | fn foo(constraints: I) where ::Item: std::fmt::Debug { | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0277]: `::Item` doesn't implement `std::fmt::Debug` @@ -25,7 +25,7 @@ LL | fn qux(_: impl std::fmt::Debug) {} = help: the trait `std::fmt::Debug` is not implemented for `::Item` help: introduce a type parameter with a trait bound instead of using `impl Trait` | -LL | fn bar(t: T, constraints: U) where T: std::fmt::Debug, ::Item: std::fmt::Debug { +LL | fn bar(t: T, constraints: I) where T: std::fmt::Debug, ::Item: std::fmt::Debug { | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0277]: `::Item` doesn't implement `std::fmt::Debug` @@ -40,7 +40,7 @@ LL | fn qux(_: impl std::fmt::Debug) {} = help: the trait `std::fmt::Debug` is not implemented for `::Item` help: introduce a type parameter with a trait bound instead of using `impl Trait` | -LL | fn baz(t: impl std::fmt::Debug, constraints: T) where ::Item: std::fmt::Debug { +LL | fn baz(t: impl std::fmt::Debug, constraints: I) where ::Item: std::fmt::Debug { | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0277]: `::Item` doesn't implement `std::fmt::Debug` @@ -55,7 +55,7 @@ LL | fn qux(_: impl std::fmt::Debug) {} = help: the trait `std::fmt::Debug` is not implemented for `::Item` help: introduce a type parameter with a trait bound instead of using `impl Trait` | -LL | fn bat(t: T, constraints: U, _: K) where ::Item: std::fmt::Debug { +LL | fn bat(t: T, constraints: U, _: I) where ::Item: std::fmt::Debug { | ^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error[E0277]: `::Item` doesn't implement `std::fmt::Debug` @@ -70,7 +70,7 @@ LL | fn qux(_: impl std::fmt::Debug) {} = help: the trait `std::fmt::Debug` is not implemented for `::Item` help: introduce a type parameter with a trait bound instead of using `impl Trait` | -LL | fn bak(constraints: T) where ::Item: std::fmt::Debug { +LL | fn bak(constraints: I) where ::Item: std::fmt::Debug { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 5 previous errors From c829db33c51259b04c976d3ab1bb4ceca8dcd86a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 5 Apr 2020 15:33:33 -0700 Subject: [PATCH 07/17] review comments --- src/librustc_hir/hir.rs | 26 --------------- .../traits/error_reporting/suggestions.rs | 32 +++++++++++++++++-- src/librustc_typeck/collect.rs | 3 +- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index f26fc402a9ac2..b719d576d6f67 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -437,32 +437,6 @@ impl GenericParam<'hir> { } } -pub trait NextTypeParamName { - fn next_type_param_name(&self, name: Option<&str>) -> String; -} - -impl NextTypeParamName for &[GenericParam<'_>] { - fn next_type_param_name(&self, name: Option<&str>) -> String { - // This is the whitelist of possible parameter names that we might suggest. - let name = name.and_then(|n| n.chars().next()).map(|c| c.to_string().to_uppercase()); - let name = name.as_ref().map(|s| s.as_str()); - let possible_names = [name.unwrap_or("T"), "T", "U", "V", "X", "Y", "Z", "A", "B", "C"]; - let used_names = self - .iter() - .filter_map(|p| match p.name { - ParamName::Plain(ident) => Some(ident.name), - _ => None, - }) - .collect::>(); - - possible_names - .iter() - .find(|n| !used_names.contains(&Symbol::intern(n))) - .unwrap_or(&"ParamName") - .to_string() - } -} - #[derive(Default)] pub struct GenericParamCount { pub lifetimes: usize, diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index 46cb297ff1353..b6f555ffe4a2d 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -10,12 +10,12 @@ use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; -use rustc_hir::{NextTypeParamName, Node}; +use rustc_hir::Node; use rustc_middle::ty::TypeckTables; use rustc_middle::ty::{ self, AdtKind, DefIdTree, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness, }; -use rustc_span::symbol::{kw, sym}; +use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::{MultiSpan, Span, DUMMY_SP}; use std::fmt; @@ -248,6 +248,8 @@ fn suggest_restriction( sugg.extend(ty_spans.into_iter().map(|s| (s, type_param_name.to_string()))); // Suggest `fn foo(t: T) where ::A: Bound`. + // FIXME: once `#![feature(associated_type_bounds)]` is stabilized, we should suggest + // `fn foo(t: impl Trait)` instead. err.multipart_suggestion( "introduce a type parameter with a trait bound instead of using `impl Trait`", sugg, @@ -1694,3 +1696,29 @@ impl<'v> Visitor<'v> for ReturnsVisitor<'v> { hir::intravisit::walk_body(self, body); } } + +pub trait NextTypeParamName { + fn next_type_param_name(&self, name: Option<&str>) -> String; +} + +impl NextTypeParamName for &[hir::GenericParam<'_>] { + fn next_type_param_name(&self, name: Option<&str>) -> String { + // This is the whitelist of possible parameter names that we might suggest. + let name = name.and_then(|n| n.chars().next()).map(|c| c.to_string().to_uppercase()); + let name = name.as_ref().map(|s| s.as_str()); + let possible_names = [name.unwrap_or("T"), "T", "U", "V", "X", "Y", "Z", "A", "B", "C"]; + let used_names = self + .iter() + .filter_map(|p| match p.name { + hir::ParamName::Plain(ident) => Some(ident.name), + _ => None, + }) + .collect::>(); + + possible_names + .iter() + .find(|n| !used_names.contains(&Symbol::intern(n))) + .unwrap_or(&"ParamName") + .to_string() + } +} diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 10fb7c32e9217..7f37a15b2ce9a 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -29,7 +29,7 @@ use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::weak_lang_items; -use rustc_hir::{GenericParamKind, NextTypeParamName, Node, Unsafety}; +use rustc_hir::{GenericParamKind, Node, Unsafety}; use rustc_middle::hir::map::blocks::FnLikeNode; use rustc_middle::hir::map::Map; use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; @@ -45,6 +45,7 @@ use rustc_session::parse::feature_err; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::{Span, DUMMY_SP}; use rustc_target::spec::abi; +use rustc_trait_selection::traits::error_reporting::suggestions::NextTypeParamName; mod type_of; From 8a03147f224369f96c2a826bc2dcbde3a6be1f10 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 9 Apr 2020 16:48:36 +0200 Subject: [PATCH 08/17] Normalize MIR locals' types for generator layout computation. --- src/librustc_mir/transform/generator.rs | 5 ++++- src/test/ui/repeat_count_const_in_async_fn.rs | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/repeat_count_const_in_async_fn.rs diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 53eec1f6dc3de..ca596f63393d7 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -720,15 +720,18 @@ fn compute_layout<'tcx>( _ => bug!(), }; + let param_env = tcx.param_env(source.def_id()); + for (local, decl) in body.local_decls.iter_enumerated() { // Ignore locals which are internal or not live if !live_locals.contains(local) || decl.internal { continue; } + let decl_ty = tcx.normalize_erasing_regions(param_env, decl.ty); // Sanity check that typeck knows about the type of locals which are // live across a suspension point - if !allowed.contains(&decl.ty) && !allowed_upvars.contains(&decl.ty) { + if !allowed.contains(&decl_ty) && !allowed_upvars.contains(&decl_ty) { span_bug!( body.span, "Broken MIR: generator contains type {} in MIR, \ diff --git a/src/test/ui/repeat_count_const_in_async_fn.rs b/src/test/ui/repeat_count_const_in_async_fn.rs new file mode 100644 index 0000000000000..ebabc3fbf10f9 --- /dev/null +++ b/src/test/ui/repeat_count_const_in_async_fn.rs @@ -0,0 +1,10 @@ +// check-pass +// edition:2018 +// compile-flags: --crate-type=lib + +pub async fn test() { + const C: usize = 4; + foo(&mut [0u8; C]).await; +} + +async fn foo(_: &mut [u8]) {} From 68b38c3bd902deccd31d59f45d5b49e58aa76a9d Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Fri, 10 Apr 2020 18:14:55 +0800 Subject: [PATCH 09/17] Normalize function signature in function casting check --- src/librustc_typeck/check/cast.rs | 5 ++++- src/test/ui/issues/issue-54094.rs | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/issues/issue-54094.rs diff --git a/src/librustc_typeck/check/cast.rs b/src/librustc_typeck/check/cast.rs index 5de0184f2bba9..38d0c42e1588b 100644 --- a/src/librustc_typeck/check/cast.rs +++ b/src/librustc_typeck/check/cast.rs @@ -536,7 +536,10 @@ impl<'a, 'tcx> CastCheck<'tcx> { match self.expr_ty.kind { ty::FnDef(..) => { // Attempt a coercion to a fn pointer type. - let f = self.expr_ty.fn_sig(fcx.tcx); + let f = fcx.normalize_associated_types_in( + self.expr.span, + &self.expr_ty.fn_sig(fcx.tcx), + ); let res = fcx.try_coerce( self.expr, self.expr_ty, diff --git a/src/test/ui/issues/issue-54094.rs b/src/test/ui/issues/issue-54094.rs new file mode 100644 index 0000000000000..57384825a35fe --- /dev/null +++ b/src/test/ui/issues/issue-54094.rs @@ -0,0 +1,14 @@ +// check-pass +trait Zoo { + type X; +} + +impl Zoo for u16 { + type X = usize; +} + +fn foo(abc: ::X) {} + +fn main() { + let x: *const u8 = foo as _; +} \ No newline at end of file From 75cc40335c5ca83ffa636ea6ff249d1934223b8b Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Fri, 10 Apr 2020 18:51:27 +0800 Subject: [PATCH 10/17] Tidy fix --- src/test/ui/issues/issue-54094.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/issues/issue-54094.rs b/src/test/ui/issues/issue-54094.rs index 57384825a35fe..ec38dc40e610a 100644 --- a/src/test/ui/issues/issue-54094.rs +++ b/src/test/ui/issues/issue-54094.rs @@ -11,4 +11,4 @@ fn foo(abc: ::X) {} fn main() { let x: *const u8 = foo as _; -} \ No newline at end of file +} From 6e70849304f3f9d15edfdc1aaeb9d2ef9937ad79 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 28 Feb 2020 22:56:37 -0500 Subject: [PATCH 11/17] tests encoding current behavior for various cases of "binding" to _. The `_` binding form is special, in that it encodes a "no-op": nothing is actually bound, and thus nothing is moved or borrowed in this scenario. Usually we do the "right" thing in all such cases. The exceptions are explicitly pointed out in this test case, so that we keep track of whether they are eventually fixed. --- .../ui/binding/issue-53114-borrow-checks.rs | 58 ++++++++++ .../binding/issue-53114-borrow-checks.stderr | 34 ++++++ .../ui/binding/issue-53114-safety-checks.rs | 51 +++++++++ .../binding/issue-53114-safety-checks.stderr | 100 ++++++++++++++++++ 4 files changed, 243 insertions(+) create mode 100644 src/test/ui/binding/issue-53114-borrow-checks.rs create mode 100644 src/test/ui/binding/issue-53114-borrow-checks.stderr create mode 100644 src/test/ui/binding/issue-53114-safety-checks.rs create mode 100644 src/test/ui/binding/issue-53114-safety-checks.stderr diff --git a/src/test/ui/binding/issue-53114-borrow-checks.rs b/src/test/ui/binding/issue-53114-borrow-checks.rs new file mode 100644 index 0000000000000..6aee2699474df --- /dev/null +++ b/src/test/ui/binding/issue-53114-borrow-checks.rs @@ -0,0 +1,58 @@ +// Issue #53114: NLL's borrow check had some deviations from the old borrow +// checker, and both had some deviations from our ideal state. This test +// captures the behavior of how `_` bindings are handled wiith respect to how we +// flag expressions that are meant to request unsafe blocks. + +struct M; + +fn let_wild_gets_moved_expr() { + let m = M; + drop(m); + let _ = m; // accepted, and want it to continue to be + + let mm = (M, M); // variation on above with `_` in substructure + let (_x, _) = mm; + let (_, _y) = mm; + let (_, _) = mm; +} + +fn match_moved_expr_to_wild() { + let m = M; + drop(m); + match m { _ => { } } // #53114: should eventually be accepted too + //~^ ERROR [E0382] + + let mm = (M, M); // variation on above with `_` in substructure + match mm { (_x, _) => { } } + match mm { (_, _y) => { } } + //~^ ERROR [E0382] + match mm { (_, _) => { } } + //~^ ERROR [E0382] +} + +fn let_wild_gets_borrowed_expr() { + let mut m = M; + let r = &mut m; + let _ = m; // accepted, and want it to continue to be + // let _x = m; // (compare with this error.) + drop(r); + + let mut mm = (M, M); // variation on above with `_` in substructure + let (r1, r2) = (&mut mm.0, &mut mm.1); + let (_, _) = mm; + drop((r1, r2)); +} + +fn match_borrowed_expr_to_wild() { + let mut m = M; + let r = &mut m; + match m { _ => {} } ; // accepted, and want it to continue to be + drop(r); + + let mut mm = (M, M); // variation on above with `_` in substructure + let (r1, r2) = (&mut mm.0, &mut mm.1); + match mm { (_, _) => { } } + drop((r1, r2)); +} + +fn main() { } diff --git a/src/test/ui/binding/issue-53114-borrow-checks.stderr b/src/test/ui/binding/issue-53114-borrow-checks.stderr new file mode 100644 index 0000000000000..18114dc09cf52 --- /dev/null +++ b/src/test/ui/binding/issue-53114-borrow-checks.stderr @@ -0,0 +1,34 @@ +error[E0382]: use of moved value: `m` + --> $DIR/issue-53114-borrow-checks.rs:22:11 + | +LL | let m = M; + | - move occurs because `m` has type `M`, which does not implement the `Copy` trait +LL | drop(m); + | - value moved here +LL | match m { _ => { } } // #53114: should eventually be accepted too + | ^ value used here after move + +error[E0382]: use of moved value: `mm` + --> $DIR/issue-53114-borrow-checks.rs:27:11 + | +LL | match mm { (_x, _) => { } } + | -- value moved here +LL | match mm { (_, _y) => { } } + | ^^ value used here after partial move + | + = note: move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait + +error[E0382]: use of moved value: `mm` + --> $DIR/issue-53114-borrow-checks.rs:29:11 + | +LL | match mm { (_, _y) => { } } + | -- value moved here +LL | +LL | match mm { (_, _) => { } } + | ^^ value used here after partial move + | + = note: move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0382`. diff --git a/src/test/ui/binding/issue-53114-safety-checks.rs b/src/test/ui/binding/issue-53114-safety-checks.rs new file mode 100644 index 0000000000000..86c863bc9a843 --- /dev/null +++ b/src/test/ui/binding/issue-53114-safety-checks.rs @@ -0,0 +1,51 @@ +// Issue #53114: NLL's borrow check had some deviations from the old borrow +// checker, and both had some deviations from our ideal state. This test +// captures the behavior of how `_` bindings are handled wiith respect to how we +// flag expressions that are meant to request unsafe blocks. + +#![feature(untagged_unions)] + +struct I(i64); +struct F(f64); + +union U { a: I, b: F } + +#[repr(packed)] +struct P { + a: &'static i8, + b: &'static u32, +} + +fn let_wild_gets_unsafe_field() { + let u1 = U { a: I(0) }; + let u2 = U { a: I(1) }; + let p = P { a: &2, b: &3 }; + let _ = &p.b; //~ WARN E0133 + //~^ WARN will become a hard error + let _ = u1.a; // #53114: should eventually signal error as well + let _ = &u2.a; //~ ERROR [E0133] + + // variation on above with `_` in substructure + let (_,) = (&p.b,); //~ WARN E0133 + //~^ WARN will become a hard error + let (_,) = (u1.a,); //~ ERROR [E0133] + let (_,) = (&u2.a,); //~ ERROR [E0133] +} + +fn match_unsafe_field_to_wild() { + let u1 = U { a: I(0) }; + let u2 = U { a: I(1) }; + let p = P { a: &2, b: &3 }; + match &p.b { _ => { } } //~ WARN E0133 + //~^ WARN will become a hard error + match u1.a { _ => { } } //~ ERROR [E0133] + match &u2.a { _ => { } } //~ ERROR [E0133] + + // variation on above with `_` in substructure + match (&p.b,) { (_,) => { } } //~ WARN E0133 + //~^ WARN will become a hard error + match (u1.a,) { (_,) => { } } //~ ERROR [E0133] + match (&u2.a,) { (_,) => { } } //~ ERROR [E0133] +} + +fn main() { } diff --git a/src/test/ui/binding/issue-53114-safety-checks.stderr b/src/test/ui/binding/issue-53114-safety-checks.stderr new file mode 100644 index 0000000000000..fc714a78bf648 --- /dev/null +++ b/src/test/ui/binding/issue-53114-safety-checks.stderr @@ -0,0 +1,100 @@ +warning: borrow of packed field is unsafe and requires unsafe function or block (error E0133) + --> $DIR/issue-53114-safety-checks.rs:23:13 + | +LL | let _ = &p.b; + | ^^^^ + | + = note: `#[warn(safe_packed_borrows)]` on by default + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/issue-53114-safety-checks.rs:26:13 + | +LL | let _ = &u2.a; + | ^^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +warning: borrow of packed field is unsafe and requires unsafe function or block (error E0133) + --> $DIR/issue-53114-safety-checks.rs:29:17 + | +LL | let (_,) = (&p.b,); + | ^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/issue-53114-safety-checks.rs:31:17 + | +LL | let (_,) = (u1.a,); + | ^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/issue-53114-safety-checks.rs:32:17 + | +LL | let (_,) = (&u2.a,); + | ^^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +warning: borrow of packed field is unsafe and requires unsafe function or block (error E0133) + --> $DIR/issue-53114-safety-checks.rs:39:11 + | +LL | match &p.b { _ => { } } + | ^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/issue-53114-safety-checks.rs:41:11 + | +LL | match u1.a { _ => { } } + | ^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/issue-53114-safety-checks.rs:42:11 + | +LL | match &u2.a { _ => { } } + | ^^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +warning: borrow of packed field is unsafe and requires unsafe function or block (error E0133) + --> $DIR/issue-53114-safety-checks.rs:45:12 + | +LL | match (&p.b,) { (_,) => { } } + | ^^^^ + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see issue #46043 + = note: fields of packed structs might be misaligned: dereferencing a misaligned pointer or even just creating a misaligned reference is undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/issue-53114-safety-checks.rs:47:12 + | +LL | match (u1.a,) { (_,) => { } } + | ^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/issue-53114-safety-checks.rs:48:12 + | +LL | match (&u2.a,) { (_,) => { } } + | ^^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error: aborting due to 7 previous errors + +For more information about this error, try `rustc --explain E0133`. From ee7a035d8c9ebb2c537abe8f03f4f3c2f6c0078c Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Thu, 5 Mar 2020 13:51:48 -0500 Subject: [PATCH 12/17] Update src/test/ui/binding/issue-53114-borrow-checks.rs Co-Authored-By: Mazdak Farrokhzad --- src/test/ui/binding/issue-53114-borrow-checks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/binding/issue-53114-borrow-checks.rs b/src/test/ui/binding/issue-53114-borrow-checks.rs index 6aee2699474df..2eb11ed84d490 100644 --- a/src/test/ui/binding/issue-53114-borrow-checks.rs +++ b/src/test/ui/binding/issue-53114-borrow-checks.rs @@ -1,6 +1,6 @@ // Issue #53114: NLL's borrow check had some deviations from the old borrow // checker, and both had some deviations from our ideal state. This test -// captures the behavior of how `_` bindings are handled wiith respect to how we +// captures the behavior of how `_` bindings are handled with respect to how we // flag expressions that are meant to request unsafe blocks. struct M; From 192d5330c40ea8ac4e3f3462c47c7cbe9b293ec9 Mon Sep 17 00:00:00 2001 From: Felix S Klock II Date: Thu, 5 Mar 2020 13:52:10 -0500 Subject: [PATCH 13/17] Update src/test/ui/binding/issue-53114-safety-checks.rs Co-Authored-By: Mazdak Farrokhzad --- src/test/ui/binding/issue-53114-safety-checks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/binding/issue-53114-safety-checks.rs b/src/test/ui/binding/issue-53114-safety-checks.rs index 86c863bc9a843..28adb7571a98b 100644 --- a/src/test/ui/binding/issue-53114-safety-checks.rs +++ b/src/test/ui/binding/issue-53114-safety-checks.rs @@ -1,6 +1,6 @@ // Issue #53114: NLL's borrow check had some deviations from the old borrow // checker, and both had some deviations from our ideal state. This test -// captures the behavior of how `_` bindings are handled wiith respect to how we +// captures the behavior of how `_` bindings are handled with respect to how we // flag expressions that are meant to request unsafe blocks. #![feature(untagged_unions)] From 1ff99b724ccafc5d0077827f6913f825be042949 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 5 Mar 2020 14:06:40 -0500 Subject: [PATCH 14/17] copy test cases to `if let` as well. --- .../ui/binding/issue-53114-borrow-checks.rs | 28 +++++++++++++++- .../binding/issue-53114-borrow-checks.stderr | 33 ++++++++++++++++++- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/test/ui/binding/issue-53114-borrow-checks.rs b/src/test/ui/binding/issue-53114-borrow-checks.rs index 2eb11ed84d490..7646472f45fac 100644 --- a/src/test/ui/binding/issue-53114-borrow-checks.rs +++ b/src/test/ui/binding/issue-53114-borrow-checks.rs @@ -2,7 +2,7 @@ // checker, and both had some deviations from our ideal state. This test // captures the behavior of how `_` bindings are handled with respect to how we // flag expressions that are meant to request unsafe blocks. - +#![allow(irrefutable_let_patterns)] struct M; fn let_wild_gets_moved_expr() { @@ -30,6 +30,20 @@ fn match_moved_expr_to_wild() { //~^ ERROR [E0382] } +fn if_let_moved_expr_to_wild() { + let m = M; + drop(m); + if let _ = m { } // #53114: should eventually be accepted too + //~^ ERROR [E0382] + + let mm = (M, M); // variation on above with `_` in substructure + if let (_x, _) = mm { } + if let (_, _y) = mm { } + //~^ ERROR [E0382] + if let (_, _) = mm { } + //~^ ERROR [E0382] +} + fn let_wild_gets_borrowed_expr() { let mut m = M; let r = &mut m; @@ -55,4 +69,16 @@ fn match_borrowed_expr_to_wild() { drop((r1, r2)); } +fn if_let_borrowed_expr_to_wild() { + let mut m = M; + let r = &mut m; + if let _ = m { } // accepted, and want it to continue to be + drop(r); + + let mut mm = (M, M); // variation on above with `_` in substructure + let (r1, r2) = (&mut mm.0, &mut mm.1); + if let (_, _) = mm { } + drop((r1, r2)); +} + fn main() { } diff --git a/src/test/ui/binding/issue-53114-borrow-checks.stderr b/src/test/ui/binding/issue-53114-borrow-checks.stderr index 18114dc09cf52..f535a66cf63ba 100644 --- a/src/test/ui/binding/issue-53114-borrow-checks.stderr +++ b/src/test/ui/binding/issue-53114-borrow-checks.stderr @@ -29,6 +29,37 @@ LL | match mm { (_, _) => { } } | = note: move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait -error: aborting due to 3 previous errors +error[E0382]: use of moved value: `m` + --> $DIR/issue-53114-borrow-checks.rs:36:16 + | +34 | let m = M; + | - move occurs because `m` has type `M`, which does not implement the `Copy` trait +35 | drop(m); + | - value moved here +36 | if let _ = m { } // #53114: should eventually be accepted too + | ^ value used here after move + +error[E0382]: use of moved value: `mm` + --> $DIR/issue-53114-borrow-checks.rs:41:22 + | +40 | if let (_x, _) = mm { } + | -- value moved here +41 | if let (_, _y) = mm { } + | ^^ value used here after partial move + | + = note: move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait + +error[E0382]: use of moved value: `mm` + --> $DIR/issue-53114-borrow-checks.rs:43:21 + | +41 | if let (_, _y) = mm { } + | -- value moved here +42 | +43 | if let (_, _) = mm { } + | ^^ value used here after partial move + | + = note: move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait + +error: aborting due to 6 previous errors For more information about this error, try `rustc --explain E0382`. From 22ea3a4476e3b2ecef218524f132b57ea14c64de Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 10 Apr 2020 18:22:24 +0200 Subject: [PATCH 15/17] --bless you --- .../ui/binding/issue-53114-borrow-checks.stderr | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/test/ui/binding/issue-53114-borrow-checks.stderr b/src/test/ui/binding/issue-53114-borrow-checks.stderr index f535a66cf63ba..2a7a721324d69 100644 --- a/src/test/ui/binding/issue-53114-borrow-checks.stderr +++ b/src/test/ui/binding/issue-53114-borrow-checks.stderr @@ -32,19 +32,19 @@ LL | match mm { (_, _) => { } } error[E0382]: use of moved value: `m` --> $DIR/issue-53114-borrow-checks.rs:36:16 | -34 | let m = M; +LL | let m = M; | - move occurs because `m` has type `M`, which does not implement the `Copy` trait -35 | drop(m); +LL | drop(m); | - value moved here -36 | if let _ = m { } // #53114: should eventually be accepted too +LL | if let _ = m { } // #53114: should eventually be accepted too | ^ value used here after move error[E0382]: use of moved value: `mm` --> $DIR/issue-53114-borrow-checks.rs:41:22 | -40 | if let (_x, _) = mm { } +LL | if let (_x, _) = mm { } | -- value moved here -41 | if let (_, _y) = mm { } +LL | if let (_, _y) = mm { } | ^^ value used here after partial move | = note: move occurs because `mm.0` has type `M`, which does not implement the `Copy` trait @@ -52,10 +52,10 @@ error[E0382]: use of moved value: `mm` error[E0382]: use of moved value: `mm` --> $DIR/issue-53114-borrow-checks.rs:43:21 | -41 | if let (_, _y) = mm { } +LL | if let (_, _y) = mm { } | -- value moved here -42 | -43 | if let (_, _) = mm { } +LL | +LL | if let (_, _) = mm { } | ^^ value used here after partial move | = note: move occurs because `mm.1` has type `M`, which does not implement the `Copy` trait From 7de9511d25e31fbb8a42d82738016ec1645b898b Mon Sep 17 00:00:00 2001 From: Ozaren Date: Thu, 9 Apr 2020 14:48:06 -0400 Subject: [PATCH 16/17] added machine hooks to track deallocations --- src/librustc_mir/interpret/machine.rs | 8 ++++++++ src/librustc_mir/interpret/memory.rs | 2 ++ 2 files changed, 10 insertions(+) diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 23e39f433f53b..fd67b088c93cf 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -254,6 +254,14 @@ pub trait Machine<'mir, 'tcx>: Sized { kind: Option>, ) -> (Cow<'b, Allocation>, Self::PointerTag); + /// Called to notify the machine before a deallocation occurs. + fn before_deallocation( + _memory_extra: &mut Self::MemoryExtra, + _id: AllocId, + ) -> InterpResult<'tcx> { + Ok(()) + } + /// Return the "base" tag for the given *global* allocation: the one that is used for direct /// accesses to this static/const/fn allocation. If `id` is not a global allocation, /// this will return an unusable tag (i.e., accesses will be UB)! diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index c16c59715e40c..539537e9de80c 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -254,6 +254,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { ); } + M::before_deallocation(&mut self.extra, ptr.alloc_id)?; + let (alloc_kind, mut alloc) = match self.alloc_map.remove(&ptr.alloc_id) { Some(alloc) => alloc, None => { From b46a7cbd1ec47df4ca13be51dcf1223b77429c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 10 Apr 2020 10:40:47 -0700 Subject: [PATCH 17/17] review comments --- .../traits/error_reporting/suggestions.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs index b6f555ffe4a2d..2087c6015f4b7 100644 --- a/src/librustc_trait_selection/traits/error_reporting/suggestions.rs +++ b/src/librustc_trait_selection/traits/error_reporting/suggestions.rs @@ -174,7 +174,7 @@ fn suggest_restriction( return; } // Given `fn foo(t: impl Trait)` where `Trait` requires assoc type `A`... - if let Some((name, fn_sig)) = + if let Some((bound_str, fn_sig)) = fn_sig.zip(projection).and_then(|(sig, p)| match p.self_ty().kind { // Shenanigans to get the `Trait` from the `impl Trait`. ty::Param(param) => { @@ -193,14 +193,14 @@ fn suggest_restriction( // but instead we choose to suggest replacing all instances of `impl Trait` with `T` // where `T: Trait`. let mut ty_spans = vec![]; - let impl_name = format!("impl {}", name); + let impl_trait_str = format!("impl {}", bound_str); for input in fn_sig.decl.inputs { if let hir::TyKind::Path(hir::QPath::Resolved( None, hir::Path { segments: [segment], .. }, )) = input.kind { - if segment.ident.as_str() == impl_name.as_str() { + if segment.ident.as_str() == impl_trait_str.as_str() { // `fn foo(t: impl Trait)` // ^^^^^^^^^^ get this to suggest `T` instead @@ -210,14 +210,14 @@ fn suggest_restriction( } } - let type_param_name = generics.params.next_type_param_name(Some(&name)); + let type_param_name = generics.params.next_type_param_name(Some(&bound_str)); // The type param `T: Trait` we will suggest to introduce. - let type_param = format!("{}: {}", type_param_name, name); + let type_param = format!("{}: {}", type_param_name, bound_str); // FIXME: modify the `trait_ref` instead of string shenanigans. // Turn `::Bar: Qux` into `::Bar: Qux`. let pred = trait_ref.without_const().to_predicate().to_string(); - let pred = pred.replace(&impl_name, &type_param_name); + let pred = pred.replace(&impl_trait_str, &type_param_name); let mut sugg = vec![ match generics .params @@ -257,10 +257,10 @@ fn suggest_restriction( ); } else { // Trivial case: `T` needs an extra bound: `T: Bound`. - let (sp, s) = + let (sp, sugg) = predicate_constraint(generics, trait_ref.without_const().to_predicate().to_string()); let appl = Applicability::MachineApplicable; - err.span_suggestion(sp, &format!("consider further restricting {}", msg), s, appl); + err.span_suggestion(sp, &format!("consider further restricting {}", msg), sugg, appl); } }