From 8e595f561045756d33e99d4a1d418d4da504d31a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 31 May 2019 22:19:30 -0700 Subject: [PATCH 1/4] Make generics always have a valid span --- src/librustc/hir/map/mod.rs | 4 --- src/librustc_typeck/check/compare_method.rs | 4 +-- src/libsyntax/parse/parser.rs | 25 ++++++++++--------- ...regions-bound-missing-bound-in-impl.stderr | 4 +-- ...type-arg-mismatch-due-to-impl-trait.stderr | 4 +-- 5 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs index fd42c6f469e1e..cbdcf9d3918b5 100644 --- a/src/librustc/hir/map/mod.rs +++ b/src/librustc/hir/map/mod.rs @@ -628,10 +628,6 @@ impl<'hir> Map<'hir> { }) } - pub fn get_generics_span(&self, id: DefId) -> Option { - self.get_generics(id).map(|generics| generics.span).filter(|sp| *sp != DUMMY_SP) - } - /// Retrieves the `Node` corresponding to `id`, returning `None` if cannot be found. pub fn find(&self, id: NodeId) -> Option> { let hir_id = self.node_to_hir_id(id); diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index 1165890fe6432..cc074b64cc01c 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -385,7 +385,7 @@ fn check_region_bounds_on_impl_method<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, // the moment, give a kind of vague error message. if trait_params != impl_params { let def_span = tcx.sess.source_map().def_span(span); - let span = tcx.hir().get_generics_span(impl_m.def_id).unwrap_or(def_span); + let span = tcx.hir().get_generics(impl_m.def_id).map(|g| g.span).unwrap_or(def_span); let mut err = struct_span_err!( tcx.sess, span, @@ -396,7 +396,7 @@ fn check_region_bounds_on_impl_method<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, err.span_label(span, "lifetimes do not match method in trait"); if let Some(sp) = tcx.hir().span_if_local(trait_m.def_id) { let def_sp = tcx.sess.source_map().def_span(sp); - let sp = tcx.hir().get_generics_span(trait_m.def_id).unwrap_or(def_sp); + let sp = tcx.hir().get_generics(trait_m.def_id).map(|g| g.span).unwrap_or(def_sp); err.span_label(sp, "lifetimes in impl do not match this method in trait"); } err.emit(); diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 746e9cad4962c..36460e75d879e 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -5050,21 +5050,22 @@ impl<'a> Parser<'a> { /// where typaramseq = ( typaram ) | ( typaram , typaramseq ) fn parse_generics(&mut self) -> PResult<'a, ast::Generics> { let span_lo = self.span; - if self.eat_lt() { + let (params, span) = if self.eat_lt() { let params = self.parse_generic_params()?; self.expect_gt()?; - Ok(ast::Generics { - params, - where_clause: WhereClause { - id: ast::DUMMY_NODE_ID, - predicates: Vec::new(), - span: DUMMY_SP, - }, - span: span_lo.to(self.prev_span), - }) + (params, span_lo.to(self.prev_span)) } else { - Ok(ast::Generics::default()) - } + (vec![], self.prev_span.between(self.span)) + }; + Ok(ast::Generics { + params, + where_clause: WhereClause { + id: ast::DUMMY_NODE_ID, + predicates: Vec::new(), + span: DUMMY_SP, + }, + span, + }) } /// Parses generic args (within a path segment) with recovery for extra leading angle brackets. diff --git a/src/test/ui/borrowck/regions-bound-missing-bound-in-impl.stderr b/src/test/ui/borrowck/regions-bound-missing-bound-in-impl.stderr index cc6d5c55bd58f..4c7c0d1a0dfa5 100644 --- a/src/test/ui/borrowck/regions-bound-missing-bound-in-impl.stderr +++ b/src/test/ui/borrowck/regions-bound-missing-bound-in-impl.stderr @@ -36,13 +36,13 @@ LL | fn wrong_bound1<'b,'c,'d:'a+'c>(self, b: Inv<'b>, c: Inv<'c>, d: Inv<'d | ^^ error[E0195]: lifetime parameters or bounds on method `wrong_bound2` do not match the trait declaration - --> $DIR/regions-bound-missing-bound-in-impl.rs:41:5 + --> $DIR/regions-bound-missing-bound-in-impl.rs:41:20 | LL | fn wrong_bound2<'b,'c,'d:'a+'b>(self, b: Inv<'b>, c: Inv<'c>, d: Inv<'d>); | ---------------- lifetimes in impl do not match this method in trait ... LL | fn wrong_bound2(self, b: Inv, c: Inv, d: Inv) { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetimes do not match method in trait + | ^ lifetimes do not match method in trait error[E0276]: impl has stricter requirements than trait --> $DIR/regions-bound-missing-bound-in-impl.rs:48:5 diff --git a/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr b/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr index af7fdde9a8ed1..3f1b10fab27f3 100644 --- a/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr +++ b/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr @@ -1,11 +1,11 @@ error[E0049]: method `foo` has 1 type parameter but its trait declaration has 0 type parameters - --> $DIR/type-arg-mismatch-due-to-impl-trait.rs:10:5 + --> $DIR/type-arg-mismatch-due-to-impl-trait.rs:10:11 | LL | fn foo(&self, t: Self::T); | -------------------------- expected 0 type parameters ... LL | fn foo(&self, t: impl Clone) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found 1 type parameter + | ^ found 1 type parameter error: aborting due to previous error From 28859472f71cea497dbea12523e69dc23daaff76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 1 Jun 2019 10:35:31 -0700 Subject: [PATCH 2/4] Point at individual type arguments on arg count mismatch --- src/librustc/hir/mod.rs | 10 +++- src/librustc_typeck/check/compare_method.rs | 56 ++++++++++++------- src/test/ui/error-codes/E0049.rs | 10 ++++ src/test/ui/error-codes/E0049.stderr | 19 +++++-- src/test/ui/issues/issue-36708.stderr | 4 +- ...type-arg-mismatch-due-to-impl-trait.stderr | 6 +- 6 files changed, 76 insertions(+), 29 deletions(-) diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index f03a8ddc90825..d9c98d60b958d 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -16,7 +16,7 @@ use crate::util::nodemap::{NodeMap, FxHashSet}; use crate::mir::mono::Linkage; use errors::FatalError; -use syntax_pos::{Span, DUMMY_SP, symbol::InternedString}; +use syntax_pos::{Span, DUMMY_SP, symbol::InternedString, MultiSpan}; use syntax::source_map::Spanned; use rustc_target::spec::abi::Abi; use syntax::ast::{self, CrateSugar, Ident, Name, NodeId, AsmDialect}; @@ -624,6 +624,14 @@ impl Generics { } None } + + pub fn spans(&self) -> MultiSpan { + if self.params.is_empty() { + self.span.into() + } else { + self.params.iter().map(|p| p.span).collect::>().into() + } + } } /// Synthetic type parameters are converted to another form during lowering; this allows diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index cc074b64cc01c..742f6ed5215cb 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -583,7 +583,7 @@ fn compare_self_type<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, fn compare_number_of_generics<'a, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_: &ty::AssocItem, - impl_span: Span, + _impl_span: Span, trait_: &ty::AssocItem, trait_span: Option, ) -> Result<(), ErrorReported> { @@ -600,17 +600,25 @@ fn compare_number_of_generics<'a, 'tcx>( if impl_count != trait_count { err_occurred = true; - let impl_hir_id = tcx.hir().as_local_hir_id(impl_.def_id).unwrap(); - let impl_item = tcx.hir().expect_impl_item(impl_hir_id); - let span = if impl_item.generics.params.is_empty() - || impl_item.generics.span.is_dummy() { // argument position impl Trait (#55374) - impl_span + let trait_spans = if let Some(trait_hir_id) = tcx.hir().as_local_hir_id(trait_.def_id) { + let trait_item = tcx.hir().expect_trait_item(trait_hir_id); + Some(if trait_item.generics.params.is_empty() { + vec![trait_item.generics.span] + } else { + trait_item.generics.params.iter().map(|p| p.span).collect::>() + }) } else { - impl_item.generics.span + trait_span.map(|s| vec![s]) }; + let impl_hir_id = tcx.hir().as_local_hir_id(impl_.def_id).unwrap(); + let impl_item = tcx.hir().expect_impl_item(impl_hir_id); + // let span = impl_item.generics.span; + let spans = impl_item.generics.spans(); + let span = spans.primary_span(); + let mut err = tcx.sess.struct_span_err_with_code( - span, + spans, &format!( "method `{}` has {} {kind} parameter{} but its trait \ declaration has {} {kind} parameter{}", @@ -626,22 +634,32 @@ fn compare_number_of_generics<'a, 'tcx>( let mut suffix = None; - if let Some(span) = trait_span { - err.span_label( - span, - format!("expected {} {} parameter{}", trait_count, kind, - if trait_count != 1 { "s" } else { "" }) - ); + if let Some(spans) = trait_spans { + let mut spans = spans.iter(); + if let Some(span) = spans.next() { + err.span_label(*span, format!( + "expected {} {} parameter{}", + trait_count, + kind, + if trait_count != 1 { "s" } else { "" }, + )); + } + for span in spans { + err.span_label(*span, ""); + } } else { suffix = Some(format!(", expected {}", trait_count)); } - err.span_label( - span, - format!("found {} {} parameter{}{}", impl_count, kind, + if let Some(span) = span { + err.span_label(span, format!( + "found {} {} parameter{}{}", + impl_count, + kind, if impl_count != 1 { "s" } else { "" }, - suffix.unwrap_or_else(|| String::new())), - ); + suffix.unwrap_or_else(|| String::new()), + )); + } err.emit(); } diff --git a/src/test/ui/error-codes/E0049.rs b/src/test/ui/error-codes/E0049.rs index c141f8a882820..3dd910019bfd0 100644 --- a/src/test/ui/error-codes/E0049.rs +++ b/src/test/ui/error-codes/E0049.rs @@ -8,5 +8,15 @@ impl Foo for Bar { fn foo(x: bool) -> Self { Bar } //~ ERROR E0049 } +trait Fuzz { + fn fuzz(x: A, y: B) -> Self; +} + +struct Baz; + +impl Fuzz for Baz { + fn fuzz(x: bool, y: bool) -> Self { Baz } //~ ERROR E0049 +} + fn main() { } diff --git a/src/test/ui/error-codes/E0049.stderr b/src/test/ui/error-codes/E0049.stderr index 7e9b9e8efb9a8..c0cd31faa90d6 100644 --- a/src/test/ui/error-codes/E0049.stderr +++ b/src/test/ui/error-codes/E0049.stderr @@ -1,12 +1,23 @@ error[E0049]: method `foo` has 0 type parameters but its trait declaration has 1 type parameter - --> $DIR/E0049.rs:8:5 + --> $DIR/E0049.rs:8:11 | LL | fn foo(x: T) -> Self; - | --------------------------------- expected 1 type parameter + | - expected 1 type parameter ... LL | fn foo(x: bool) -> Self { Bar } - | ^^^^^^^^^^^^^^^^^^^^^^^ found 0 type parameters + | ^ found 0 type parameters -error: aborting due to previous error +error[E0049]: method `fuzz` has 0 type parameters but its trait declaration has 2 type parameters + --> $DIR/E0049.rs:18:12 + | +LL | fn fuzz(x: A, y: B) -> Self; + | - - + | | + | expected 2 type parameters +... +LL | fn fuzz(x: bool, y: bool) -> Self { Baz } + | ^ found 0 type parameters + +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0049`. diff --git a/src/test/ui/issues/issue-36708.stderr b/src/test/ui/issues/issue-36708.stderr index 835094c4fdc5e..140f19f1ff774 100644 --- a/src/test/ui/issues/issue-36708.stderr +++ b/src/test/ui/issues/issue-36708.stderr @@ -1,8 +1,8 @@ error[E0049]: method `foo` has 1 type parameter but its trait declaration has 0 type parameters - --> $DIR/issue-36708.rs:8:11 + --> $DIR/issue-36708.rs:8:12 | LL | fn foo() {} - | ^^^ found 1 type parameter, expected 0 + | ^ found 1 type parameter, expected 0 error: aborting due to previous error diff --git a/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr b/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr index 3f1b10fab27f3..953284735553c 100644 --- a/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr +++ b/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr @@ -1,11 +1,11 @@ error[E0049]: method `foo` has 1 type parameter but its trait declaration has 0 type parameters - --> $DIR/type-arg-mismatch-due-to-impl-trait.rs:10:11 + --> $DIR/type-arg-mismatch-due-to-impl-trait.rs:10:22 | LL | fn foo(&self, t: Self::T); - | -------------------------- expected 0 type parameters + | - expected 0 type parameters ... LL | fn foo(&self, t: impl Clone) {} - | ^ found 1 type parameter + | ^^^^^^^^^^ found 1 type parameter error: aborting due to previous error From 0754c8461182252f5f86eca09845ece04b533696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 1 Jun 2019 11:33:38 -0700 Subject: [PATCH 3/4] Explain that `impl Trait` introduces an implicit type argument --- src/librustc_typeck/check/compare_method.rs | 35 +++++++++++++++---- .../type-arg-mismatch-due-to-impl-trait.rs | 1 + ...type-arg-mismatch-due-to-impl-trait.stderr | 5 ++- 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index 742f6ed5215cb..fb83f877ccce2 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -600,20 +600,37 @@ fn compare_number_of_generics<'a, 'tcx>( if impl_count != trait_count { err_occurred = true; - let trait_spans = if let Some(trait_hir_id) = tcx.hir().as_local_hir_id(trait_.def_id) { + let ( + trait_spans, + impl_trait_spans, + ) = if let Some(trait_hir_id) = tcx.hir().as_local_hir_id(trait_.def_id) { let trait_item = tcx.hir().expect_trait_item(trait_hir_id); - Some(if trait_item.generics.params.is_empty() { - vec![trait_item.generics.span] + if trait_item.generics.params.is_empty() { + (Some(vec![trait_item.generics.span]), vec![]) } else { - trait_item.generics.params.iter().map(|p| p.span).collect::>() - }) + let arg_spans: Vec = trait_item.generics.params.iter() + .map(|p| p.span) + .collect(); + let impl_trait_spans: Vec = trait_item.generics.params.iter() + .filter_map(|p| if !trait_item.generics.span.overlaps(p.span) { + Some(p.span) + } else { + None + }).collect(); + (Some(arg_spans), impl_trait_spans) + } } else { - trait_span.map(|s| vec![s]) + (trait_span.map(|s| vec![s]), vec![]) }; let impl_hir_id = tcx.hir().as_local_hir_id(impl_.def_id).unwrap(); let impl_item = tcx.hir().expect_impl_item(impl_hir_id); - // let span = impl_item.generics.span; + let impl_item_impl_trait_spans: Vec = impl_item.generics.params.iter() + .filter_map(|p| if !impl_item.generics.span.overlaps(p.span) { + Some(p.span) + } else { + None + }).collect(); let spans = impl_item.generics.spans(); let span = spans.primary_span(); @@ -661,6 +678,10 @@ fn compare_number_of_generics<'a, 'tcx>( )); } + for span in impl_trait_spans.iter().chain(impl_item_impl_trait_spans.iter()) { + err.span_label(*span, "`impl Trait` introduces an implicit type parameter"); + } + err.emit(); } } diff --git a/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.rs b/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.rs index 4a71932d1df18..ecfa5c69e2f03 100644 --- a/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.rs +++ b/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.rs @@ -10,6 +10,7 @@ impl Foo for u32 { fn foo(&self, t: impl Clone) {} //~^ ERROR method `foo` has 1 type parameter but its trait declaration has 0 type parameters //~| NOTE found 1 type parameter +//~| NOTE `impl Trait` introduces an implicit type parameter } fn main() {} diff --git a/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr b/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr index 953284735553c..30322f88cca42 100644 --- a/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr +++ b/src/test/ui/issues/type-arg-mismatch-due-to-impl-trait.stderr @@ -5,7 +5,10 @@ LL | fn foo(&self, t: Self::T); | - expected 0 type parameters ... LL | fn foo(&self, t: impl Clone) {} - | ^^^^^^^^^^ found 1 type parameter + | ^^^^^^^^^^ + | | + | found 1 type parameter + | `impl Trait` introduces an implicit type parameter error: aborting due to previous error From 31918d6eef4d5c6459fc4e16e4ddedf9d8ab698d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 1 Jun 2019 15:39:12 -0700 Subject: [PATCH 4/4] review comments: use param kind type to identify impl Trait --- src/librustc_typeck/check/compare_method.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index fb83f877ccce2..b4548ac70911e 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -612,10 +612,11 @@ fn compare_number_of_generics<'a, 'tcx>( .map(|p| p.span) .collect(); let impl_trait_spans: Vec = trait_item.generics.params.iter() - .filter_map(|p| if !trait_item.generics.span.overlaps(p.span) { - Some(p.span) - } else { - None + .filter_map(|p| match p.kind { + GenericParamKind::Type { + synthetic: Some(hir::SyntheticTyParamKind::ImplTrait), .. + } => Some(p.span), + _ => None, }).collect(); (Some(arg_spans), impl_trait_spans) } @@ -626,10 +627,11 @@ fn compare_number_of_generics<'a, 'tcx>( let impl_hir_id = tcx.hir().as_local_hir_id(impl_.def_id).unwrap(); let impl_item = tcx.hir().expect_impl_item(impl_hir_id); let impl_item_impl_trait_spans: Vec = impl_item.generics.params.iter() - .filter_map(|p| if !impl_item.generics.span.overlaps(p.span) { - Some(p.span) - } else { - None + .filter_map(|p| match p.kind { + GenericParamKind::Type { + synthetic: Some(hir::SyntheticTyParamKind::ImplTrait), .. + } => Some(p.span), + _ => None, }).collect(); let spans = impl_item.generics.spans(); let span = spans.primary_span();