From ce982a35e18e08b81a1524435f390b291f6e32b8 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Mon, 31 May 2021 19:58:15 -0500 Subject: [PATCH 1/5] debug for suggest_unsized_bound_if_applicable --- .../src/traits/error_reporting/mod.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 19c3385dd4cbc..f522e272ecf66 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1763,7 +1763,10 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { ) => (pred, item_def_id, span), _ => return, }; - + debug!( + "suggest_unsized_bound_if_applicable: pred={:?} item_def_id={:?} span={:?}", + pred, item_def_id, span + ); let node = match ( self.tcx.hir().get_if_local(item_def_id), Some(pred.def_id()) == self.tcx.lang_items().sized_trait(), @@ -1775,6 +1778,11 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { Some(generics) => generics, None => return, }; + debug!("suggest_unsized_bound_if_applicable: generics.params={:?}", generics.params); + debug!( + "suggest_unsized_bound_if_applicable: generics.where_clause={:?}", + generics.where_clause + ); for param in generics.params { if param.span != span || param.bounds.iter().any(|bound| { @@ -1784,6 +1792,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { { continue; } + debug!("suggest_unsized_bound_if_applicable: param={:?}", param); match node { hir::Node::Item( item @@ -1895,6 +1904,7 @@ impl<'v> Visitor<'v> for FindTypeParam { if path.segments.len() == 1 && path.segments[0].ident.name == self.param => { if !self.nested { + debug!("FindTypeParam::visit_ty: ty={:?}", ty); self.invalid_spans.push(ty.span); } } From 2862f08b79c73233e338a063d7a58e642903c9f5 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Mon, 31 May 2021 20:46:58 -0500 Subject: [PATCH 2/5] factor out maybe_suggest_unsized_generics --- .../src/traits/error_reporting/mod.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index f522e272ecf66..0441e3a1693e1 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1088,6 +1088,13 @@ trait InferCtxtPrivExt<'tcx> { obligation: &PredicateObligation<'tcx>, ); + fn maybe_suggest_unsized_generics( + &self, + err: &mut DiagnosticBuilder<'tcx>, + span: Span, + node: Node<'hir>, + ); + fn is_recursive_obligation( &self, obligated_types: &mut Vec<&ty::TyS<'tcx>>, @@ -1774,6 +1781,15 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { (Some(node), true) => node, _ => return, }; + self.maybe_suggest_unsized_generics(err, span, node); + } + + fn maybe_suggest_unsized_generics( + &self, + err: &mut DiagnosticBuilder<'tcx>, + span: Span, + node: Node<'hir>, + ) { let generics = match node.generics() { Some(generics) => generics, None => return, @@ -1792,7 +1808,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { { continue; } - debug!("suggest_unsized_bound_if_applicable: param={:?}", param); + debug!("maybe_suggest_unsized_generics: param={:?}", param); match node { hir::Node::Item( item From 437b2026e1bfcb1f28d838f3a2a24b23f0401b53 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Tue, 1 Jun 2021 12:35:24 -0500 Subject: [PATCH 3/5] factor out maybe_indirection_for_unsized --- .../src/traits/error_reporting/mod.rs | 79 +++++++++++-------- 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 0441e3a1693e1..4d0e8c5022949 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -16,6 +16,8 @@ use rustc_errors::{pluralize, struct_span_err, Applicability, DiagnosticBuilder, use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::Visitor; +use rustc_hir::GenericParam; +use rustc_hir::Item; use rustc_hir::Node; use rustc_middle::mir::abstract_const::NotConstEvaluatable; use rustc_middle::ty::error::ExpectedFound; @@ -1095,6 +1097,13 @@ trait InferCtxtPrivExt<'tcx> { node: Node<'hir>, ); + fn maybe_indirection_for_unsized( + &self, + err: &mut DiagnosticBuilder<'tcx>, + item: &'hir Item<'hir>, + param: &'hir GenericParam<'hir>, + ) -> bool; + fn is_recursive_obligation( &self, obligated_types: &mut Vec<&ty::TyS<'tcx>>, @@ -1821,38 +1830,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { .. }, ) => { - // Suggesting `T: ?Sized` is only valid in an ADT if `T` is only used in a - // borrow. `struct S<'a, T: ?Sized>(&'a T);` is valid, `struct S(T);` - // is not. - let mut visitor = FindTypeParam { - param: param.name.ident().name, - invalid_spans: vec![], - nested: false, - }; - visitor.visit_item(item); - if !visitor.invalid_spans.is_empty() { - let mut multispan: MultiSpan = param.span.into(); - multispan.push_span_label( - param.span, - format!("this could be changed to `{}: ?Sized`...", param.name.ident()), - ); - for sp in visitor.invalid_spans { - multispan.push_span_label( - sp, - format!( - "...if indirection were used here: `Box<{}>`", - param.name.ident(), - ), - ); - } - err.span_help( - multispan, - &format!( - "you could relax the implicit `Sized` bound on `{T}` if it were \ - used through indirection like `&{T}` or `Box<{T}>`", - T = param.name.ident(), - ), - ); + if self.maybe_indirection_for_unsized(err, item, param) { return; } } @@ -1872,6 +1850,43 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { } } + fn maybe_indirection_for_unsized( + &self, + err: &mut DiagnosticBuilder<'tcx>, + item: &'hir Item<'hir>, + param: &'hir GenericParam<'hir>, + ) -> bool { + // Suggesting `T: ?Sized` is only valid in an ADT if `T` is only used in a + // borrow. `struct S<'a, T: ?Sized>(&'a T);` is valid, `struct S(T);` + // is not. + let mut visitor = + FindTypeParam { param: param.name.ident().name, invalid_spans: vec![], nested: false }; + visitor.visit_item(item); + if !visitor.invalid_spans.is_empty() { + let mut multispan: MultiSpan = param.span.into(); + multispan.push_span_label( + param.span, + format!("this could be changed to `{}: ?Sized`...", param.name.ident()), + ); + for sp in visitor.invalid_spans { + multispan.push_span_label( + sp, + format!("...if indirection were used here: `Box<{}>`", param.name.ident()), + ); + } + err.span_help( + multispan, + &format!( + "you could relax the implicit `Sized` bound on `{T}` if it were \ + used through indirection like `&{T}` or `Box<{T}>`", + T = param.name.ident(), + ), + ); + return true; + } + false + } + fn is_recursive_obligation( &self, obligated_types: &mut Vec<&ty::TyS<'tcx>>, From 69f0dc69a45889f58c5e13fabdd8c8eabfd604a1 Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Tue, 1 Jun 2021 20:15:12 -0500 Subject: [PATCH 4/5] deindent unsized suggestions Move stuff out of loops. Use early returns. --- .../src/traits/error_reporting/mod.rs | 126 +++++++++--------- 1 file changed, 63 insertions(+), 63 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 4d0e8c5022949..1ebdb8ac26cb8 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1803,51 +1803,51 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { Some(generics) => generics, None => return, }; - debug!("suggest_unsized_bound_if_applicable: generics.params={:?}", generics.params); - debug!( - "suggest_unsized_bound_if_applicable: generics.where_clause={:?}", - generics.where_clause - ); - for param in generics.params { - if param.span != span - || param.bounds.iter().any(|bound| { - bound.trait_ref().and_then(|trait_ref| trait_ref.trait_def_id()) - == self.tcx.lang_items().sized_trait() - }) - { - continue; - } - debug!("maybe_suggest_unsized_generics: param={:?}", param); - match node { - hir::Node::Item( - item - @ - hir::Item { - kind: - hir::ItemKind::Enum(..) - | hir::ItemKind::Struct(..) - | hir::ItemKind::Union(..), - .. - }, - ) => { - if self.maybe_indirection_for_unsized(err, item, param) { - return; - } + let sized_trait = self.tcx.lang_items().sized_trait(); + debug!("maybe_suggest_unsized_generics: generics.params={:?}", generics.params); + debug!("maybe_suggest_unsized_generics: generics.where_clause={:?}", generics.where_clause); + let param = generics + .params + .iter() + .filter(|param| param.span == span) + .filter(|param| { + param + .bounds + .iter() + .all(|bound| bound.trait_ref().and_then(|tr| tr.trait_def_id()) != sized_trait) + }) + .next(); + let param = match param { + Some(param) => param, + _ => return, + }; + debug!("maybe_suggest_unsized_generics: param={:?}", param); + match node { + hir::Node::Item( + item + @ + hir::Item { + kind: + hir::ItemKind::Enum(..) | hir::ItemKind::Struct(..) | hir::ItemKind::Union(..), + .. + }, + ) => { + if self.maybe_indirection_for_unsized(err, item, param) { + return; } - _ => {} } - let (span, separator) = match param.bounds { - [] => (span.shrink_to_hi(), ":"), - [.., bound] => (bound.span().shrink_to_hi(), " +"), - }; - err.span_suggestion_verbose( - span, - "consider relaxing the implicit `Sized` restriction", - format!("{} ?Sized", separator), - Applicability::MachineApplicable, - ); - return; - } + _ => {} + }; + let (span, separator) = match param.bounds { + [] => (span.shrink_to_hi(), ":"), + [.., bound] => (bound.span().shrink_to_hi(), " +"), + }; + err.span_suggestion_verbose( + span, + "consider relaxing the implicit `Sized` restriction", + format!("{} ?Sized", separator), + Applicability::MachineApplicable, + ); } fn maybe_indirection_for_unsized( @@ -1862,29 +1862,29 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { let mut visitor = FindTypeParam { param: param.name.ident().name, invalid_spans: vec![], nested: false }; visitor.visit_item(item); - if !visitor.invalid_spans.is_empty() { - let mut multispan: MultiSpan = param.span.into(); + if visitor.invalid_spans.is_empty() { + return false; + } + let mut multispan: MultiSpan = param.span.into(); + multispan.push_span_label( + param.span, + format!("this could be changed to `{}: ?Sized`...", param.name.ident()), + ); + for sp in visitor.invalid_spans { multispan.push_span_label( - param.span, - format!("this could be changed to `{}: ?Sized`...", param.name.ident()), + sp, + format!("...if indirection were used here: `Box<{}>`", param.name.ident()), ); - for sp in visitor.invalid_spans { - multispan.push_span_label( - sp, - format!("...if indirection were used here: `Box<{}>`", param.name.ident()), - ); - } - err.span_help( - multispan, - &format!( - "you could relax the implicit `Sized` bound on `{T}` if it were \ - used through indirection like `&{T}` or `Box<{T}>`", - T = param.name.ident(), - ), - ); - return true; } - false + err.span_help( + multispan, + &format!( + "you could relax the implicit `Sized` bound on `{T}` if it were \ + used through indirection like `&{T}` or `Box<{T}>`", + T = param.name.ident(), + ), + ); + true } fn is_recursive_obligation( From 3252432c27c8ff14f52ce04fa08c6bb73ce40ebe Mon Sep 17 00:00:00 2001 From: Taylor Yu Date: Wed, 2 Jun 2021 16:06:50 -0500 Subject: [PATCH 5/5] improve comments for unsized suggestions --- .../rustc_trait_selection/src/traits/error_reporting/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs index 1ebdb8ac26cb8..648cd483bf6c4 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs @@ -1811,6 +1811,8 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { .iter() .filter(|param| param.span == span) .filter(|param| { + // Check that none of the explicit trait bounds is `Sized`. Assume that an explicit + // `Sized` bound is there intentionally and we don't need to suggest relaxing it. param .bounds .iter() @@ -1827,6 +1829,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { item @ hir::Item { + // Only suggest indirection for uses of type parameters in ADTs. kind: hir::ItemKind::Enum(..) | hir::ItemKind::Struct(..) | hir::ItemKind::Union(..), .. @@ -1838,6 +1841,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { } _ => {} }; + // Didn't add an indirection suggestion, so add a general suggestion to relax `Sized`. let (span, separator) = match param.bounds { [] => (span.shrink_to_hi(), ":"), [.., bound] => (bound.span().shrink_to_hi(), " +"), @@ -1858,7 +1862,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> { ) -> bool { // Suggesting `T: ?Sized` is only valid in an ADT if `T` is only used in a // borrow. `struct S<'a, T: ?Sized>(&'a T);` is valid, `struct S(T);` - // is not. + // is not. Look for invalid "bare" parameter uses, and suggest using indirection. let mut visitor = FindTypeParam { param: param.name.ident().name, invalid_spans: vec![], nested: false }; visitor.visit_item(item);