From 0f530ecb6875540f5cf5032ea7df54a38d01ab1c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Esteban=20K=C3=BCber?= Date: Sat, 23 Nov 2019 21:44:28 -0800 Subject: [PATCH] review comments --- src/librustc/hir/mod.rs | 77 +------------------ src/librustc/traits/error_reporting.rs | 77 ++++++++++++++++++- .../borrow_check/conflict_errors.rs | 4 +- 3 files changed, 81 insertions(+), 77 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 0b79f624709..66bb3a8d883 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -16,9 +16,9 @@ use crate::ty::query::Providers; use crate::util::nodemap::{NodeMap, FxHashSet}; -use errors::{Applicability, DiagnosticBuilder, FatalError}; +use errors::FatalError; use syntax_pos::{Span, DUMMY_SP, MultiSpan}; -use syntax::source_map::{Spanned, SourceMap}; +use syntax::source_map::Spanned; use syntax::ast::{self, CrateSugar, Ident, Name, NodeId, AsmDialect}; use syntax::ast::{Attribute, Label, LitKind, StrStyle, FloatTy, IntTy, UintTy}; pub use syntax::ast::{Mutability, Constness, Unsafety, Movability, CaptureBy}; @@ -644,79 +644,6 @@ pub fn spans(&self) -> MultiSpan { self.params.iter().map(|p| p.span).collect::>().into() } } - - /// Suggest restricting a type param with a new bound. - pub fn suggest_constraining_type_param( - &self, - err: &mut DiagnosticBuilder<'_>, - param_name: &str, - constraint: &str, - source_map: &SourceMap, - span: Span, - ) -> bool { - let restrict_msg = "consider further restricting this bound"; - if let Some(param) = self.params.iter().filter(|p| { - p.name.ident().as_str() == param_name - }).next() { - if param_name.starts_with("impl ") { - // `impl Trait` in argument: - // `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}` - err.span_suggestion( - param.span, - restrict_msg, - // `impl CurrentTrait + MissingTrait` - format!("{} + {}", param_name, constraint), - Applicability::MachineApplicable, - ); - } else if self.where_clause.predicates.is_empty() && - param.bounds.is_empty() - { - // If there are no bounds whatsoever, suggest adding a constraint - // to the type parameter: - // `fn foo(t: T) {}` → `fn foo(t: T) {}` - err.span_suggestion( - param.span, - "consider restricting this bound", - format!("{}: {}", param_name, constraint), - Applicability::MachineApplicable, - ); - } else if !self.where_clause.predicates.is_empty() { - // There is a `where` clause, so suggest expanding it: - // `fn foo(t: T) where T: Debug {}` → - // `fn foo(t: T) where T: Debug, T: Trait {}` - err.span_suggestion( - self.where_clause.span().unwrap().shrink_to_hi(), - &format!("consider further restricting type parameter `{}`", param_name), - format!(", {}: {}", param_name, constraint), - Applicability::MachineApplicable, - ); - } else { - // If there is no `where` clause lean towards constraining to the - // type parameter: - // `fn foo(t: T, x: X) {}` → `fn foo(t: T) {}` - // `fn foo(t: T) {}` → `fn foo(t: T) {}` - let sp = param.span.with_hi(span.hi()); - let span = source_map.span_through_char(sp, ':'); - if sp != param.span && sp != span { - // Only suggest if we have high certainty that the span - // covers the colon in `foo`. - err.span_suggestion( - span, - restrict_msg, - format!("{}: {} + ", param_name, constraint), - Applicability::MachineApplicable, - ); - } else { - err.span_label( - param.span, - &format!("consider adding a `where {}: {}` bound", param_name, constraint), - ); - } - } - return true; - } - false - } } /// Synthetic type parameters are converted to another form during lowering; this allows diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index db7c5c3f778..6e723cdc999 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -39,6 +39,7 @@ use syntax::symbol::{sym, kw}; use syntax_pos::{DUMMY_SP, Span, ExpnKind, MultiSpan}; use rustc::hir::def_id::LOCAL_CRATE; +use syntax_pos::source_map::SourceMap; use rustc_error_codes::*; @@ -1189,7 +1190,8 @@ fn suggest_restricting_param_bound( // Missing generic type parameter bound. let param_name = self_ty.to_string(); let constraint = trait_ref.to_string(); - if generics.suggest_constraining_type_param( + if suggest_constraining_type_param( + generics, &mut err, ¶m_name, &constraint, @@ -2497,3 +2499,76 @@ pub fn from_expected_ty(t: Ty<'_>, span: Option) -> ArgKind { } } } + +/// Suggest restricting a type param with a new bound. +pub fn suggest_constraining_type_param( + generics: &hir::Generics, + err: &mut DiagnosticBuilder<'_>, + param_name: &str, + constraint: &str, + source_map: &SourceMap, + span: Span, +) -> bool { + let restrict_msg = "consider further restricting this bound"; + if let Some(param) = generics.params.iter().filter(|p| { + p.name.ident().as_str() == param_name + }).next() { + if param_name.starts_with("impl ") { + // `impl Trait` in argument: + // `fn foo(x: impl Trait) {}` → `fn foo(t: impl Trait + Trait2) {}` + err.span_suggestion( + param.span, + restrict_msg, + // `impl CurrentTrait + MissingTrait` + format!("{} + {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } else if generics.where_clause.predicates.is_empty() && + param.bounds.is_empty() + { + // If there are no bounds whatsoever, suggest adding a constraint + // to the type parameter: + // `fn foo(t: T) {}` → `fn foo(t: T) {}` + err.span_suggestion( + param.span, + "consider restricting this bound", + format!("{}: {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } else if !generics.where_clause.predicates.is_empty() { + // There is a `where` clause, so suggest expanding it: + // `fn foo(t: T) where T: Debug {}` → + // `fn foo(t: T) where T: Debug, T: Trait {}` + err.span_suggestion( + generics.where_clause.span().unwrap().shrink_to_hi(), + &format!("consider further restricting type parameter `{}`", param_name), + format!(", {}: {}", param_name, constraint), + Applicability::MachineApplicable, + ); + } else { + // If there is no `where` clause lean towards constraining to the + // type parameter: + // `fn foo(t: T, x: X) {}` → `fn foo(t: T) {}` + // `fn foo(t: T) {}` → `fn foo(t: T) {}` + let sp = param.span.with_hi(span.hi()); + let span = source_map.span_through_char(sp, ':'); + if sp != param.span && sp != span { + // Only suggest if we have high certainty that the span + // covers the colon in `foo`. + err.span_suggestion( + span, + restrict_msg, + format!("{}: {} + ", param_name, constraint), + Applicability::MachineApplicable, + ); + } else { + err.span_label( + param.span, + &format!("consider adding a `where {}: {}` bound", param_name, constraint), + ); + } + } + return true; + } + false +} diff --git a/src/librustc_mir/borrow_check/conflict_errors.rs b/src/librustc_mir/borrow_check/conflict_errors.rs index e9065abcebe..48f8ad9bbd8 100644 --- a/src/librustc_mir/borrow_check/conflict_errors.rs +++ b/src/librustc_mir/borrow_check/conflict_errors.rs @@ -7,6 +7,7 @@ PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind, TerminatorKind, VarBindingForm, }; use rustc::ty::{self, Ty}; +use rustc::traits::error_reporting::suggest_constraining_type_param; use rustc_data_structures::fx::FxHashSet; use rustc_index::vec::Idx; use rustc_errors::{Applicability, DiagnosticBuilder}; @@ -233,7 +234,8 @@ pub(super) fn report_use_of_moved_or_uninitialized( let generics = tcx.generics_of(self.mir_def_id); let param = generics.type_param(¶m_ty, tcx); let generics = tcx.hir().get_generics(self.mir_def_id).unwrap(); - generics.suggest_constraining_type_param( + suggest_constraining_type_param( + generics, &mut err, ¶m.name.as_str(), "Copy", -- 2.44.0