From 3827b6451c8cfbab9ec1f21bb385eabfede74022 Mon Sep 17 00:00:00 2001 From: Tom Farmer <tfarmer7980@gmail.com> Date: Sat, 9 Oct 2021 13:13:15 +0100 Subject: [PATCH 1/7] Update invalid crate attributes, add help message tidy run update invalid crate attributes, improve error update test outputs de-capitalise error update tests Update invalid crate attributes, add help message Update - generate span without using BytePos Add correct dependancies Update - generate suggestion without BytePos Tidy run update tests Generate Suggestion without BytePos Add all builtin attributes add err builtin inner attr at top of crate fix tests add err builtin inner attr at top of crate tidy fix add err builtin inner attr at top of crate --- compiler/rustc_passes/src/check_attr.rs | 36 ++++++-- src/test/ui/derives/issue-36617.rs | 13 +++ src/test/ui/derives/issue-36617.stderr | 89 ++++++++++++++++++- .../issue-43106-gating-of-bench.rs | 2 +- .../issue-43106-gating-of-bench.stderr | 13 ++- ...43106-gating-of-builtin-attrs-error.stderr | 30 +++++++ .../issue-43106-gating-of-test.rs | 2 +- .../issue-43106-gating-of-test.stderr | 13 ++- src/test/ui/imports/issue-28134.rs | 1 + src/test/ui/imports/issue-28134.stderr | 13 ++- .../ui/span/issue-43927-non-ADT-derive.rs | 1 + .../ui/span/issue-43927-non-ADT-derive.stderr | 13 ++- 12 files changed, 213 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 7c0b3a57da9..49e6a7df103 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -1953,7 +1953,12 @@ fn is_c_like_enum(item: &Item<'_>) -> bool { } } +// FIXME: Fix "Cannot determine resolution" error and remove built-in macros +// from this check. fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) { + // Check for builtin attributes at the crate level + // which were unsuccessfully resolved due to cannot determine + // resolution for the attribute macro error. const ATTRS_TO_CHECK: &[Symbol] = &[ sym::macro_export, sym::repr, @@ -1961,20 +1966,39 @@ fn check_invalid_crate_level_attr(tcx: TyCtxt<'_>, attrs: &[Attribute]) { sym::automatically_derived, sym::start, sym::rustc_main, + sym::derive, + sym::test, + sym::test_case, + sym::global_allocator, + sym::bench, ]; for attr in attrs { - for attr_to_check in ATTRS_TO_CHECK { - if attr.has_name(*attr_to_check) { - tcx.sess - .struct_span_err( + // This function should only be called with crate attributes + // which are inner attributes always but lets check to make sure + if attr.style == AttrStyle::Inner { + for attr_to_check in ATTRS_TO_CHECK { + if attr.has_name(*attr_to_check) { + let mut err = tcx.sess.struct_span_err( attr.span, &format!( "`{}` attribute cannot be used at crate level", attr_to_check.to_ident_string() ), - ) - .emit(); + ); + // Only emit an error with a suggestion if we can create a + // string out of the attribute span + if let Ok(src) = tcx.sess.source_map().span_to_snippet(attr.span) { + let replacement = src.replace("#!", "#"); + err.span_suggestion_verbose( + attr.span, + "perhaps you meant to use an outer attribute", + replacement, + rustc_errors::Applicability::MachineApplicable, + ); + } + err.emit() + } } } } diff --git a/src/test/ui/derives/issue-36617.rs b/src/test/ui/derives/issue-36617.rs index 08fc82e91f6..08f293d2ebb 100644 --- a/src/test/ui/derives/issue-36617.rs +++ b/src/test/ui/derives/issue-36617.rs @@ -1,3 +1,16 @@ #![derive(Copy)] //~ ERROR cannot determine resolution for the attribute macro `derive` +//~^ ERROR `derive` attribute cannot be used at crate level + +#![test]//~ ERROR cannot determine resolution for the attribute macro `test` +//~^ ERROR `test` attribute cannot be used at crate level + +#![test_case]//~ ERROR cannot determine resolution for the attribute macro `test_case` +//~^ ERROR `test_case` attribute cannot be used at crate level + +#![bench]//~ ERROR cannot determine resolution for the attribute macro `bench` +//~^ ERROR `bench` attribute cannot be used at crate level + +#![global_allocator]//~ ERROR cannot determine resolution for the attribute macro `global_allocator` +//~^ ERROR `global_allocator` attribute cannot be used at crate level fn main() {} diff --git a/src/test/ui/derives/issue-36617.stderr b/src/test/ui/derives/issue-36617.stderr index 0716764b427..9cc0a29b065 100644 --- a/src/test/ui/derives/issue-36617.stderr +++ b/src/test/ui/derives/issue-36617.stderr @@ -6,5 +6,92 @@ LL | #![derive(Copy)] | = note: import resolution is stuck, try simplifying macro imports -error: aborting due to previous error +error: cannot determine resolution for the attribute macro `test` + --> $DIR/issue-36617.rs:4:4 + | +LL | #![test] + | ^^^^ + | + = note: import resolution is stuck, try simplifying macro imports + +error: cannot determine resolution for the attribute macro `test_case` + --> $DIR/issue-36617.rs:7:4 + | +LL | #![test_case] + | ^^^^^^^^^ + | + = note: import resolution is stuck, try simplifying macro imports + +error: cannot determine resolution for the attribute macro `bench` + --> $DIR/issue-36617.rs:10:4 + | +LL | #![bench] + | ^^^^^ + | + = note: import resolution is stuck, try simplifying macro imports + +error: cannot determine resolution for the attribute macro `global_allocator` + --> $DIR/issue-36617.rs:13:4 + | +LL | #![global_allocator] + | ^^^^^^^^^^^^^^^^ + | + = note: import resolution is stuck, try simplifying macro imports + +error: `derive` attribute cannot be used at crate level + --> $DIR/issue-36617.rs:1:1 + | +LL | #![derive(Copy)] + | ^^^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[derive(Copy)] + | ~~~~~~~~~~~~~~~ + +error: `test` attribute cannot be used at crate level + --> $DIR/issue-36617.rs:4:1 + | +LL | #![test] + | ^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[test] + | ~~~~~~~ + +error: `test_case` attribute cannot be used at crate level + --> $DIR/issue-36617.rs:7:1 + | +LL | #![test_case] + | ^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[test_case] + | ~~~~~~~~~~~~ + +error: `bench` attribute cannot be used at crate level + --> $DIR/issue-36617.rs:10:1 + | +LL | #![bench] + | ^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[bench] + | ~~~~~~~~ + +error: `global_allocator` attribute cannot be used at crate level + --> $DIR/issue-36617.rs:13:1 + | +LL | #![global_allocator] + | ^^^^^^^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[global_allocator] + | ~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 10 previous errors diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-bench.rs b/src/test/ui/feature-gates/issue-43106-gating-of-bench.rs index 31eee88d1fa..796325b79af 100644 --- a/src/test/ui/feature-gates/issue-43106-gating-of-bench.rs +++ b/src/test/ui/feature-gates/issue-43106-gating-of-bench.rs @@ -6,5 +6,5 @@ #![bench = "4100"] //~^ ERROR cannot determine resolution for the attribute macro `bench` - +//~^^ ERROR `bench` attribute cannot be used at crate level fn main() {} diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-bench.stderr b/src/test/ui/feature-gates/issue-43106-gating-of-bench.stderr index d0305c5160f..6b332211942 100644 --- a/src/test/ui/feature-gates/issue-43106-gating-of-bench.stderr +++ b/src/test/ui/feature-gates/issue-43106-gating-of-bench.stderr @@ -6,5 +6,16 @@ LL | #![bench = "4100"] | = note: import resolution is stuck, try simplifying macro imports -error: aborting due to previous error +error: `bench` attribute cannot be used at crate level + --> $DIR/issue-43106-gating-of-bench.rs:7:1 + | +LL | #![bench = "4100"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[bench = "4100"] + | + +error: aborting due to 2 previous errors diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.stderr b/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.stderr index 9e2e2d4137d..f94ec7d4704 100644 --- a/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.stderr +++ b/src/test/ui/feature-gates/issue-43106-gating-of-builtin-attrs-error.stderr @@ -129,36 +129,66 @@ error: `macro_export` attribute cannot be used at crate level | LL | #![macro_export] | ^^^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[macro_export] + | error: `rustc_main` attribute cannot be used at crate level --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:14:1 | LL | #![rustc_main] | ^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[rustc_main] + | ~~~~~~~~~~~~~ error: `start` attribute cannot be used at crate level --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:16:1 | LL | #![start] | ^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[start] + | error: `repr` attribute cannot be used at crate level --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:18:1 | LL | #![repr()] | ^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[repr()] + | error: `path` attribute cannot be used at crate level --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:20:1 | LL | #![path = "3800"] | ^^^^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[path = "3800"] + | error: `automatically_derived` attribute cannot be used at crate level --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:22:1 | LL | #![automatically_derived] | ^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[automatically_derived] + | error[E0518]: attribute should be applied to function or closure --> $DIR/issue-43106-gating-of-builtin-attrs-error.rs:36:17 diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-test.rs b/src/test/ui/feature-gates/issue-43106-gating-of-test.rs index ee3fe712e36..39835c9268e 100644 --- a/src/test/ui/feature-gates/issue-43106-gating-of-test.rs +++ b/src/test/ui/feature-gates/issue-43106-gating-of-test.rs @@ -3,5 +3,5 @@ #![allow(soft_unstable)] #![test = "4200"] //~^ ERROR cannot determine resolution for the attribute macro `test` - +//~^^ ERROR `test` attribute cannot be used at crate level fn main() {} diff --git a/src/test/ui/feature-gates/issue-43106-gating-of-test.stderr b/src/test/ui/feature-gates/issue-43106-gating-of-test.stderr index 335af5e7905..300a9966dd8 100644 --- a/src/test/ui/feature-gates/issue-43106-gating-of-test.stderr +++ b/src/test/ui/feature-gates/issue-43106-gating-of-test.stderr @@ -6,5 +6,16 @@ LL | #![test = "4200"] | = note: import resolution is stuck, try simplifying macro imports -error: aborting due to previous error +error: `test` attribute cannot be used at crate level + --> $DIR/issue-43106-gating-of-test.rs:4:1 + | +LL | #![test = "4200"] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[test = "4200"] + | + +error: aborting due to 2 previous errors diff --git a/src/test/ui/imports/issue-28134.rs b/src/test/ui/imports/issue-28134.rs index 1ed2d330b51..ef2a5d634a6 100644 --- a/src/test/ui/imports/issue-28134.rs +++ b/src/test/ui/imports/issue-28134.rs @@ -2,3 +2,4 @@ #![allow(soft_unstable)] #![test] //~ ERROR cannot determine resolution for the attribute macro `test` +//~^ ERROR 4:1: 4:9: `test` attribute cannot be used at crate level diff --git a/src/test/ui/imports/issue-28134.stderr b/src/test/ui/imports/issue-28134.stderr index 8ed4d015f32..33cb53f202a 100644 --- a/src/test/ui/imports/issue-28134.stderr +++ b/src/test/ui/imports/issue-28134.stderr @@ -6,5 +6,16 @@ LL | #![test] | = note: import resolution is stuck, try simplifying macro imports -error: aborting due to previous error +error: `test` attribute cannot be used at crate level + --> $DIR/issue-28134.rs:4:1 + | +LL | #![test] + | ^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[test] + | ~~~~~~~ + +error: aborting due to 2 previous errors diff --git a/src/test/ui/span/issue-43927-non-ADT-derive.rs b/src/test/ui/span/issue-43927-non-ADT-derive.rs index 840c12e16e1..935bfa001bf 100644 --- a/src/test/ui/span/issue-43927-non-ADT-derive.rs +++ b/src/test/ui/span/issue-43927-non-ADT-derive.rs @@ -1,5 +1,6 @@ #![derive(Debug, PartialEq, Eq)] // should be an outer attribute! //~^ ERROR cannot determine resolution for the attribute macro `derive` +//~^^ ERROR `derive` attribute cannot be used at crate level struct DerivedOn; fn main() {} diff --git a/src/test/ui/span/issue-43927-non-ADT-derive.stderr b/src/test/ui/span/issue-43927-non-ADT-derive.stderr index 9ef81c5150a..e3ae37e3689 100644 --- a/src/test/ui/span/issue-43927-non-ADT-derive.stderr +++ b/src/test/ui/span/issue-43927-non-ADT-derive.stderr @@ -6,5 +6,16 @@ LL | #![derive(Debug, PartialEq, Eq)] // should be an outer attribute! | = note: import resolution is stuck, try simplifying macro imports -error: aborting due to previous error +error: `derive` attribute cannot be used at crate level + --> $DIR/issue-43927-non-ADT-derive.rs:1:1 + | +LL | #![derive(Debug, PartialEq, Eq)] // should be an outer attribute! + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: perhaps you meant to use an outer attribute + | +LL | #[derive(Debug, PartialEq, Eq)] // should be an outer attribute! + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 2 previous errors From e70105f9719351990fe026c1c0777b1d685ed744 Mon Sep 17 00:00:00 2001 From: Esteban Kuber <esteban@kuber.com.ar> Date: Wed, 3 Nov 2021 04:19:06 +0000 Subject: [PATCH 2/7] Keep spans for generics in `#[derive(_)]` desugaring Keep the spans for generics coming from a `derive`d Item, so that errors and suggestions have better detail. Fix #84003. --- .../src/deriving/generic/mod.rs | 26 ++--- .../src/deriving/generic/ty.rs | 16 +-- compiler/rustc_hir/src/hir.rs | 4 +- .../rustc_resolve/src/late/diagnostics.rs | 16 +-- src/test/ui/issues/issue-38821.stderr | 4 + src/test/ui/issues/issue-50480.rs | 11 +- src/test/ui/issues/issue-50480.stderr | 89 ++++++++++++--- ...ime-used-in-debug-macro-issue-70152.stderr | 3 +- .../derive-macro-missing-bounds.rs | 89 +++++++++++++++ .../derive-macro-missing-bounds.stderr | 107 ++++++++++++++++++ 10 files changed, 312 insertions(+), 53 deletions(-) create mode 100644 src/test/ui/suggestions/derive-macro-missing-bounds.rs create mode 100644 src/test/ui/suggestions/derive-macro-missing-bounds.stderr diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 994a74a5a9b..0cdb86ea475 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -567,8 +567,10 @@ impl<'a> TraitDef<'a> { }) }); - let Generics { mut params, mut where_clause, span } = + let Generics { mut params, mut where_clause, .. } = self.generics.to_generics(cx, self.span, type_ident, generics); + where_clause.span = generics.where_clause.span; + let span = generics.span; // Create the generic parameters params.extend(generics.params.iter().map(|param| match ¶m.kind { @@ -589,7 +591,7 @@ impl<'a> TraitDef<'a> { param.bounds.iter().cloned() ).collect(); - cx.typaram(self.span, param.ident, vec![], bounds, None) + cx.typaram(param.ident.span, param.ident, vec![], bounds, None) } GenericParamKind::Const { ty, kw_span, .. } => { let const_nodefault_kind = GenericParamKind::Const { @@ -610,7 +612,7 @@ impl<'a> TraitDef<'a> { match *clause { ast::WherePredicate::BoundPredicate(ref wb) => { ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate { - span: self.span, + span: wb.span, bound_generic_params: wb.bound_generic_params.clone(), bounded_ty: wb.bounded_ty.clone(), bounds: wb.bounds.to_vec(), @@ -618,7 +620,7 @@ impl<'a> TraitDef<'a> { } ast::WherePredicate::RegionPredicate(ref rb) => { ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate { - span: self.span, + span: rb.span, lifetime: rb.lifetime, bounds: rb.bounds.to_vec(), }) @@ -626,7 +628,7 @@ impl<'a> TraitDef<'a> { ast::WherePredicate::EqPredicate(ref we) => { ast::WherePredicate::EqPredicate(ast::WhereEqPredicate { id: ast::DUMMY_NODE_ID, - span: self.span, + span: we.span, lhs_ty: we.lhs_ty.clone(), rhs_ty: we.rhs_ty.clone(), }) @@ -691,13 +693,13 @@ impl<'a> TraitDef<'a> { .iter() .map(|param| match param.kind { GenericParamKind::Lifetime { .. } => { - GenericArg::Lifetime(cx.lifetime(self.span, param.ident)) + GenericArg::Lifetime(cx.lifetime(param.ident.span, param.ident)) } GenericParamKind::Type { .. } => { - GenericArg::Type(cx.ty_ident(self.span, param.ident)) + GenericArg::Type(cx.ty_ident(param.ident.span, param.ident)) } GenericParamKind::Const { .. } => { - GenericArg::Const(cx.const_ident(self.span, param.ident)) + GenericArg::Const(cx.const_ident(param.ident.span, param.ident)) } }) .collect(); @@ -1556,11 +1558,9 @@ impl<'a> TraitDef<'a> { let is_tuple = matches!(struct_def, ast::VariantData::Tuple(..)); match (just_spans.is_empty(), named_idents.is_empty()) { - (false, false) => cx.span_bug( - self.span, - "a struct with named and unnamed \ - fields in generic `derive`", - ), + (false, false) => { + cx.span_bug(self.span, "a struct with named and unnamed fields in generic `derive`") + } // named fields (_, false) => Named(named_idents), // unnamed fields diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs b/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs index 00d75be4399..7a418003250 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/ty.rs @@ -211,14 +211,6 @@ fn mk_ty_param( cx.typaram(span, Ident::new(name, span), attrs.to_owned(), bounds, None) } -fn mk_generics(params: Vec<ast::GenericParam>, span: Span) -> Generics { - Generics { - params, - where_clause: ast::WhereClause { has_where_token: false, predicates: Vec::new(), span }, - span, - } -} - /// Bounds on type parameters. #[derive(Clone)] pub struct Bounds { @@ -236,7 +228,7 @@ impl Bounds { self_ty: Ident, self_generics: &Generics, ) -> Generics { - let generic_params = self + let params = self .bounds .iter() .map(|t| { @@ -245,7 +237,11 @@ impl Bounds { }) .collect(); - mk_generics(generic_params, span) + Generics { + params, + where_clause: ast::WhereClause { has_where_token: false, predicates: Vec::new(), span }, + span, + } } } diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 61a96564a4c..69f770265dd 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -17,8 +17,7 @@ use rustc_index::vec::IndexVec; use rustc_macros::HashStable_Generic; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{def_id::LocalDefId, BytePos}; -use rustc_span::{MultiSpan, Span, DUMMY_SP}; +use rustc_span::{def_id::LocalDefId, BytePos, MultiSpan, Span, DUMMY_SP}; use rustc_target::asm::InlineAsmRegOrRegClass; use rustc_target::spec::abi::Abi; @@ -529,7 +528,6 @@ impl GenericParam<'hir> { pub fn bounds_span(&self) -> Option<Span> { self.bounds.iter().fold(None, |span, bound| { let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span()); - Some(span) }) } diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 69697f275e1..4e071d69e36 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1803,7 +1803,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { ); err.span_label(lifetime_ref.span, "undeclared lifetime"); let mut suggests_in_band = false; - let mut suggest_note = true; + let mut suggested_spans = vec![]; for missing in &self.missing_named_lifetime_spots { match missing { MissingLifetimeSpot::Generics(generics) => { @@ -1821,6 +1821,10 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { suggests_in_band = true; (generics.span, format!("<{}>", lifetime_ref)) }; + if suggested_spans.contains(&span) { + continue; + } + suggested_spans.push(span); if !span.from_expansion() { err.span_suggestion( span, @@ -1828,16 +1832,6 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { sugg, Applicability::MaybeIncorrect, ); - } else if suggest_note { - suggest_note = false; // Avoid displaying the same help multiple times. - err.span_label( - span, - &format!( - "lifetime `{}` is missing in item created through this procedural \ - macro", - lifetime_ref, - ), - ); } } MissingLifetimeSpot::HigherRanked { span, span_type } => { diff --git a/src/test/ui/issues/issue-38821.stderr b/src/test/ui/issues/issue-38821.stderr index e53a543f3a0..3a8b69224d3 100644 --- a/src/test/ui/issues/issue-38821.stderr +++ b/src/test/ui/issues/issue-38821.stderr @@ -10,6 +10,10 @@ note: required because of the requirements on the impl of `IntoNullable` for `<C LL | impl<T: NotNull> IntoNullable for T { | ^^^^^^^^^^^^ ^ = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider further restricting the associated type + | +LL | Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: aborting due to previous error diff --git a/src/test/ui/issues/issue-50480.rs b/src/test/ui/issues/issue-50480.rs index deb63872f69..10597caf5b2 100644 --- a/src/test/ui/issues/issue-50480.rs +++ b/src/test/ui/issues/issue-50480.rs @@ -1,8 +1,17 @@ #[derive(Clone, Copy)] //~^ ERROR the trait `Copy` may not be implemented for this type -struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); +struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); //~^ ERROR cannot find type `NotDefined` in this scope //~| ERROR cannot find type `NotDefined` in this scope +//~| ERROR cannot find type `N` in this scope +//~| ERROR cannot find type `N` in this scope +//~| ERROR `i32` is not an iterator + +#[derive(Clone, Copy)] +//~^ ERROR the trait `Copy` may not be implemented for this type +struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); +//~^ ERROR cannot find type `NotDefined` in this scope +//~| ERROR cannot find type `N` in this scope //~| ERROR `i32` is not an iterator fn main() {} diff --git a/src/test/ui/issues/issue-50480.stderr b/src/test/ui/issues/issue-50480.stderr index 15f38c89267..0bb1f9ae035 100644 --- a/src/test/ui/issues/issue-50480.stderr +++ b/src/test/ui/issues/issue-50480.stderr @@ -1,20 +1,61 @@ -error[E0412]: cannot find type `NotDefined` in this scope +error[E0412]: cannot find type `N` in this scope --> $DIR/issue-50480.rs:3:12 | -LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); - | ^^^^^^^^^^ not found in this scope +LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | -^ not found in this scope + | | + | help: you might be missing a type parameter: `<N>` error[E0412]: cannot find type `NotDefined` in this scope + --> $DIR/issue-50480.rs:3:15 + | +LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | ^^^^^^^^^^ not found in this scope + +error[E0412]: cannot find type `N` in this scope --> $DIR/issue-50480.rs:3:12 | -LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); - | ^^^^^^^^^^ not found in this scope +LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | -^ not found in this scope + | | + | help: you might be missing a type parameter: `<N>` + +error[E0412]: cannot find type `NotDefined` in this scope + --> $DIR/issue-50480.rs:3:15 + | +LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | - ^^^^^^^^^^ not found in this scope + | | + | help: you might be missing a type parameter: `<NotDefined>` + +error[E0412]: cannot find type `N` in this scope + --> $DIR/issue-50480.rs:12:18 + | +LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | - ^ + | | + | similarly named type parameter `T` defined here + | +help: a type parameter with a similar name exists + | +LL | struct Bar<T>(T, T, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | ~ +help: you might be missing a type parameter + | +LL | struct Bar<T, N>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | +++ + +error[E0412]: cannot find type `NotDefined` in this scope + --> $DIR/issue-50480.rs:12:21 + | +LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | ^^^^^^^^^^ not found in this scope error[E0277]: `i32` is not an iterator - --> $DIR/issue-50480.rs:3:24 + --> $DIR/issue-50480.rs:3:27 | -LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); - | ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator +LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator | = help: the trait `Iterator` is not implemented for `i32` = note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end` @@ -25,14 +66,36 @@ error[E0204]: the trait `Copy` may not be implemented for this type LL | #[derive(Clone, Copy)] | ^^^^ LL | -LL | struct Foo(NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); - | -------- ------ this field does not implement `Copy` - | | - | this field does not implement `Copy` +LL | struct Foo(N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | -------- ------ this field does not implement `Copy` + | | + | this field does not implement `Copy` | = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 4 previous errors +error[E0277]: `i32` is not an iterator + --> $DIR/issue-50480.rs:12:33 + | +LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | ^^^^^^^^^^^^^^^^^^^^^^^ `i32` is not an iterator + | + = help: the trait `Iterator` is not implemented for `i32` + = note: if you want to iterate between `start` until a value `end`, use the exclusive range syntax `start..end` or the inclusive range syntax `start..=end` + +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/issue-50480.rs:10:17 + | +LL | #[derive(Clone, Copy)] + | ^^^^ +LL | +LL | struct Bar<T>(T, N, NotDefined, <i32 as Iterator>::Item, Vec<i32>, String); + | -------- ------ this field does not implement `Copy` + | | + | this field does not implement `Copy` + | + = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 10 previous errors Some errors have detailed explanations: E0204, E0277, E0412. For more information about an error, try `rustc --explain E0204`. diff --git a/src/test/ui/lifetimes/undeclared-lifetime-used-in-debug-macro-issue-70152.stderr b/src/test/ui/lifetimes/undeclared-lifetime-used-in-debug-macro-issue-70152.stderr index e18d725faef..a2086895234 100644 --- a/src/test/ui/lifetimes/undeclared-lifetime-used-in-debug-macro-issue-70152.stderr +++ b/src/test/ui/lifetimes/undeclared-lifetime-used-in-debug-macro-issue-70152.stderr @@ -11,9 +11,8 @@ LL | a: &'b str, error[E0261]: use of undeclared lifetime name `'b` --> $DIR/undeclared-lifetime-used-in-debug-macro-issue-70152.rs:3:9 | -LL | #[derive(Eq, PartialEq)] - | -- lifetime `'b` is missing in item created through this procedural macro LL | struct Test { + | - help: consider introducing lifetime `'b` here: `<'b>` LL | a: &'b str, | ^^ undeclared lifetime | diff --git a/src/test/ui/suggestions/derive-macro-missing-bounds.rs b/src/test/ui/suggestions/derive-macro-missing-bounds.rs new file mode 100644 index 00000000000..56c218f97eb --- /dev/null +++ b/src/test/ui/suggestions/derive-macro-missing-bounds.rs @@ -0,0 +1,89 @@ +mod a { + use std::fmt::{Debug, Formatter, Result}; + struct Inner<T>(T); + + impl Debug for Inner<()> { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + todo!() + } + } + + #[derive(Debug)] + struct Outer<T>(Inner<T>); //~ ERROR `a::Inner<T>` doesn't implement `Debug` +} + +mod b { + use std::fmt::{Debug, Formatter, Result}; + struct Inner<T>(T); + + impl<T: Debug> Debug for Inner<T> { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + todo!() + } + } + + #[derive(Debug)] + struct Outer<T>(Inner<T>); +} + +mod c { + use std::fmt::{Debug, Formatter, Result}; + struct Inner<T>(T); + trait Trait {} + + impl<T: Debug + Trait> Debug for Inner<T> { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + todo!() + } + } + + #[derive(Debug)] + struct Outer<T>(Inner<T>); //~ ERROR the trait bound `T: c::Trait` is not satisfied +} + +mod d { + use std::fmt::{Debug, Formatter, Result}; + struct Inner<T>(T); + trait Trait {} + + impl<T> Debug for Inner<T> where T: Debug, T: Trait { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + todo!() + } + } + + #[derive(Debug)] + struct Outer<T>(Inner<T>); //~ ERROR the trait bound `T: d::Trait` is not satisfied +} + +mod e { + use std::fmt::{Debug, Formatter, Result}; + struct Inner<T>(T); + trait Trait {} + + impl<T> Debug for Inner<T> where T: Debug + Trait { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + todo!() + } + } + + #[derive(Debug)] + struct Outer<T>(Inner<T>); //~ ERROR the trait bound `T: e::Trait` is not satisfied +} + +mod f { + use std::fmt::{Debug, Formatter, Result}; + struct Inner<T>(T); + trait Trait {} + + impl<T: Debug> Debug for Inner<T> where T: Trait { + fn fmt(&self, f: &mut Formatter<'_>) -> Result { + todo!() + } + } + + #[derive(Debug)] + struct Outer<T>(Inner<T>); //~ ERROR the trait bound `T: f::Trait` is not satisfied +} + +fn main() {} diff --git a/src/test/ui/suggestions/derive-macro-missing-bounds.stderr b/src/test/ui/suggestions/derive-macro-missing-bounds.stderr new file mode 100644 index 00000000000..39678105930 --- /dev/null +++ b/src/test/ui/suggestions/derive-macro-missing-bounds.stderr @@ -0,0 +1,107 @@ +error[E0277]: `a::Inner<T>` doesn't implement `Debug` + --> $DIR/derive-macro-missing-bounds.rs:12:21 + | +LL | #[derive(Debug)] + | ----- in this derive macro expansion +LL | struct Outer<T>(Inner<T>); + | ^^^^^^^^ `a::Inner<T>` cannot be formatted using `{:?}` + | + = help: the trait `Debug` is not implemented for `a::Inner<T>` + = note: add `#[derive(Debug)]` to `a::Inner<T>` or manually `impl Debug for a::Inner<T>` + = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement + | +LL | struct Outer<T>(Inner<T>) where a::Inner<T>: Debug; + | ++++++++++++++++++++++++ + +error[E0277]: the trait bound `T: c::Trait` is not satisfied + --> $DIR/derive-macro-missing-bounds.rs:41:21 + | +LL | #[derive(Debug)] + | ----- in this derive macro expansion +LL | struct Outer<T>(Inner<T>); + | ^^^^^^^^ the trait `c::Trait` is not implemented for `T` + | +note: required because of the requirements on the impl of `Debug` for `c::Inner<T>` + --> $DIR/derive-macro-missing-bounds.rs:34:28 + | +LL | impl<T: Debug + Trait> Debug for Inner<T> { + | ^^^^^ ^^^^^^^^ + = note: 1 redundant requirement hidden + = note: required because of the requirements on the impl of `Debug` for `&c::Inner<T>` + = note: required for the cast to the object type `dyn Debug` + = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider further restricting this bound + | +LL | #[derive(Debug + c::Trait)] + | ++++++++++ + +error[E0277]: the trait bound `T: d::Trait` is not satisfied + --> $DIR/derive-macro-missing-bounds.rs:56:21 + | +LL | #[derive(Debug)] + | ----- in this derive macro expansion +LL | struct Outer<T>(Inner<T>); + | ^^^^^^^^ the trait `d::Trait` is not implemented for `T` + | +note: required because of the requirements on the impl of `Debug` for `d::Inner<T>` + --> $DIR/derive-macro-missing-bounds.rs:49:13 + | +LL | impl<T> Debug for Inner<T> where T: Debug, T: Trait { + | ^^^^^ ^^^^^^^^ + = note: 1 redundant requirement hidden + = note: required because of the requirements on the impl of `Debug` for `&d::Inner<T>` + = note: required for the cast to the object type `dyn Debug` + = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider further restricting this bound + | +LL | #[derive(Debug + d::Trait)] + | ++++++++++ + +error[E0277]: the trait bound `T: e::Trait` is not satisfied + --> $DIR/derive-macro-missing-bounds.rs:71:21 + | +LL | #[derive(Debug)] + | ----- in this derive macro expansion +LL | struct Outer<T>(Inner<T>); + | ^^^^^^^^ the trait `e::Trait` is not implemented for `T` + | +note: required because of the requirements on the impl of `Debug` for `e::Inner<T>` + --> $DIR/derive-macro-missing-bounds.rs:64:13 + | +LL | impl<T> Debug for Inner<T> where T: Debug + Trait { + | ^^^^^ ^^^^^^^^ + = note: 1 redundant requirement hidden + = note: required because of the requirements on the impl of `Debug` for `&e::Inner<T>` + = note: required for the cast to the object type `dyn Debug` + = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider further restricting this bound + | +LL | #[derive(Debug + e::Trait)] + | ++++++++++ + +error[E0277]: the trait bound `T: f::Trait` is not satisfied + --> $DIR/derive-macro-missing-bounds.rs:86:21 + | +LL | #[derive(Debug)] + | ----- in this derive macro expansion +LL | struct Outer<T>(Inner<T>); + | ^^^^^^^^ the trait `f::Trait` is not implemented for `T` + | +note: required because of the requirements on the impl of `Debug` for `f::Inner<T>` + --> $DIR/derive-macro-missing-bounds.rs:79:20 + | +LL | impl<T: Debug> Debug for Inner<T> where T: Trait { + | ^^^^^ ^^^^^^^^ + = note: 1 redundant requirement hidden + = note: required because of the requirements on the impl of `Debug` for `&f::Inner<T>` + = note: required for the cast to the object type `dyn Debug` + = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider further restricting this bound + | +LL | #[derive(Debug + f::Trait)] + | ++++++++++ + +error: aborting due to 5 previous errors + +For more information about this error, try `rustc --explain E0277`. From fa3eebb26e103a8f0fa81b378e8c8525a80baf32 Mon Sep 17 00:00:00 2001 From: Esteban Kuber <esteban@kuber.com.ar> Date: Wed, 17 Nov 2021 23:35:38 +0000 Subject: [PATCH 3/7] Modify `bounds_span` to ignore bounds coming from a `derive` macro --- compiler/rustc_hir/src/hir.rs | 15 +++++++++--- .../derive-macro-missing-bounds.stderr | 24 +++++++++---------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 69f770265dd..d5132ad5bdf 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -17,7 +17,7 @@ use rustc_index::vec::IndexVec; use rustc_macros::HashStable_Generic; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{def_id::LocalDefId, BytePos, MultiSpan, Span, DUMMY_SP}; +use rustc_span::{def_id::LocalDefId, BytePos, ExpnKind, MacroKind, MultiSpan, Span, DUMMY_SP}; use rustc_target::asm::InlineAsmRegOrRegClass; use rustc_target::spec::abi::Abi; @@ -527,8 +527,17 @@ pub struct GenericParam<'hir> { impl GenericParam<'hir> { pub fn bounds_span(&self) -> Option<Span> { self.bounds.iter().fold(None, |span, bound| { - let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span()); - Some(span) + if let ExpnKind::Macro(MacroKind::Derive, _) = + bound.span().ctxt().outer_expn_data().kind + { + // We ignore bounds that come from exclusively from a `#[derive(_)]`, because we + // can't really point at them, and we sometimes use this method to get a span + // appropriate for suggestions. + span + } else { + let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span()); + Some(span) + } }) } } diff --git a/src/test/ui/suggestions/derive-macro-missing-bounds.stderr b/src/test/ui/suggestions/derive-macro-missing-bounds.stderr index 39678105930..7a4f7e209c1 100644 --- a/src/test/ui/suggestions/derive-macro-missing-bounds.stderr +++ b/src/test/ui/suggestions/derive-macro-missing-bounds.stderr @@ -31,10 +31,10 @@ LL | impl<T: Debug + Trait> Debug for Inner<T> { = note: required because of the requirements on the impl of `Debug` for `&c::Inner<T>` = note: required for the cast to the object type `dyn Debug` = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider further restricting this bound +help: consider restricting type parameter `T` | -LL | #[derive(Debug + c::Trait)] - | ++++++++++ +LL | struct Outer<T: c::Trait>(Inner<T>); + | ++++++++++ error[E0277]: the trait bound `T: d::Trait` is not satisfied --> $DIR/derive-macro-missing-bounds.rs:56:21 @@ -53,10 +53,10 @@ LL | impl<T> Debug for Inner<T> where T: Debug, T: Trait { = note: required because of the requirements on the impl of `Debug` for `&d::Inner<T>` = note: required for the cast to the object type `dyn Debug` = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider further restricting this bound +help: consider restricting type parameter `T` | -LL | #[derive(Debug + d::Trait)] - | ++++++++++ +LL | struct Outer<T: d::Trait>(Inner<T>); + | ++++++++++ error[E0277]: the trait bound `T: e::Trait` is not satisfied --> $DIR/derive-macro-missing-bounds.rs:71:21 @@ -75,10 +75,10 @@ LL | impl<T> Debug for Inner<T> where T: Debug + Trait { = note: required because of the requirements on the impl of `Debug` for `&e::Inner<T>` = note: required for the cast to the object type `dyn Debug` = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider further restricting this bound +help: consider restricting type parameter `T` | -LL | #[derive(Debug + e::Trait)] - | ++++++++++ +LL | struct Outer<T: e::Trait>(Inner<T>); + | ++++++++++ error[E0277]: the trait bound `T: f::Trait` is not satisfied --> $DIR/derive-macro-missing-bounds.rs:86:21 @@ -97,10 +97,10 @@ LL | impl<T: Debug> Debug for Inner<T> where T: Trait { = note: required because of the requirements on the impl of `Debug` for `&f::Inner<T>` = note: required for the cast to the object type `dyn Debug` = note: this error originates in the derive macro `Debug` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider further restricting this bound +help: consider restricting type parameter `T` | -LL | #[derive(Debug + f::Trait)] - | ++++++++++ +LL | struct Outer<T: f::Trait>(Inner<T>); + | ++++++++++ error: aborting due to 5 previous errors From 8bee2b88c075a2f007a7d5762727a840477c0893 Mon Sep 17 00:00:00 2001 From: Esteban Kuber <esteban@kuber.com.ar> Date: Wed, 17 Nov 2021 23:47:47 +0000 Subject: [PATCH 4/7] Remove some unnecessarily verbose code --- .../src/deriving/generic/mod.rs | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 0cdb86ea475..58b6ef9f9f9 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -609,28 +609,13 @@ impl<'a> TraitDef<'a> { // and similarly for where clauses where_clause.predicates.extend(generics.where_clause.predicates.iter().map(|clause| { - match *clause { - ast::WherePredicate::BoundPredicate(ref wb) => { - ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate { - span: wb.span, - bound_generic_params: wb.bound_generic_params.clone(), - bounded_ty: wb.bounded_ty.clone(), - bounds: wb.bounds.to_vec(), - }) - } - ast::WherePredicate::RegionPredicate(ref rb) => { - ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate { - span: rb.span, - lifetime: rb.lifetime, - bounds: rb.bounds.to_vec(), - }) - } - ast::WherePredicate::EqPredicate(ref we) => { + match clause { + ast::WherePredicate::BoundPredicate(_) + | ast::WherePredicate::RegionPredicate(_) => clause.clone(), + ast::WherePredicate::EqPredicate(we) => { ast::WherePredicate::EqPredicate(ast::WhereEqPredicate { id: ast::DUMMY_NODE_ID, - span: we.span, - lhs_ty: we.lhs_ty.clone(), - rhs_ty: we.rhs_ty.clone(), + ..we.clone() }) } } From 962b2197a5d5796ee030a05f534ee0fde8ce07bb Mon Sep 17 00:00:00 2001 From: Esteban Kuber <esteban@kuber.com.ar> Date: Sat, 20 Nov 2021 18:46:36 +0000 Subject: [PATCH 5/7] Annotate `derive`d spans and move span suggestion code * Annotate `derive`d spans from the user's code with the appropciate context * Add `Span::can_be_used_for_suggestion` to query if the underlying span at the users' code --- .../src/deriving/generic/mod.rs | 135 ++++++++++-------- compiler/rustc_hir/src/hir.rs | 31 ++-- compiler/rustc_middle/src/ty/diagnostics.rs | 4 +- .../rustc_resolve/src/late/diagnostics.rs | 4 +- compiler/rustc_span/src/lib.rs | 10 ++ .../src/traits/error_reporting/suggestions.rs | 6 +- compiler/rustc_typeck/src/collect.rs | 8 +- src/test/ui/issues/issue-38821.stderr | 4 +- 8 files changed, 112 insertions(+), 90 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 58b6ef9f9f9..1427a2aada3 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -570,7 +570,8 @@ impl<'a> TraitDef<'a> { let Generics { mut params, mut where_clause, .. } = self.generics.to_generics(cx, self.span, type_ident, generics); where_clause.span = generics.where_clause.span; - let span = generics.span; + let ctxt = self.span.ctxt(); + let span = generics.span.with_ctxt(ctxt); // Create the generic parameters params.extend(generics.params.iter().map(|param| match ¶m.kind { @@ -591,12 +592,12 @@ impl<'a> TraitDef<'a> { param.bounds.iter().cloned() ).collect(); - cx.typaram(param.ident.span, param.ident, vec![], bounds, None) + cx.typaram(param.ident.span.with_ctxt(ctxt), param.ident, vec![], bounds, None) } GenericParamKind::Const { ty, kw_span, .. } => { let const_nodefault_kind = GenericParamKind::Const { ty: ty.clone(), - kw_span: *kw_span, + kw_span: kw_span.with_ctxt(ctxt), // We can't have default values inside impl block default: None, @@ -610,11 +611,25 @@ impl<'a> TraitDef<'a> { // and similarly for where clauses where_clause.predicates.extend(generics.where_clause.predicates.iter().map(|clause| { match clause { - ast::WherePredicate::BoundPredicate(_) - | ast::WherePredicate::RegionPredicate(_) => clause.clone(), + ast::WherePredicate::BoundPredicate(wb) => { + let span = wb.span.with_ctxt(ctxt); + ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate { + span, + ..wb.clone() + }) + } + ast::WherePredicate::RegionPredicate(wr) => { + let span = wr.span.with_ctxt(ctxt); + ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate { + span, + ..wr.clone() + }) + } ast::WherePredicate::EqPredicate(we) => { + let span = we.span.with_ctxt(ctxt); ast::WherePredicate::EqPredicate(ast::WhereEqPredicate { id: ast::DUMMY_NODE_ID, + span, ..we.clone() }) } @@ -678,13 +693,13 @@ impl<'a> TraitDef<'a> { .iter() .map(|param| match param.kind { GenericParamKind::Lifetime { .. } => { - GenericArg::Lifetime(cx.lifetime(param.ident.span, param.ident)) + GenericArg::Lifetime(cx.lifetime(param.ident.span.with_ctxt(ctxt), param.ident)) } GenericParamKind::Type { .. } => { - GenericArg::Type(cx.ty_ident(param.ident.span, param.ident)) + GenericArg::Type(cx.ty_ident(param.ident.span.with_ctxt(ctxt), param.ident)) } GenericParamKind::Const { .. } => { - GenericArg::Const(cx.const_ident(param.ident.span, param.ident)) + GenericArg::Const(cx.const_ident(param.ident.span.with_ctxt(ctxt), param.ident)) } }) .collect(); @@ -832,16 +847,17 @@ impl<'a> MethodDef<'a> { nonself_args: &[P<Expr>], fields: &SubstructureFields<'_>, ) -> P<Expr> { + let span = trait_.span; let substructure = Substructure { type_ident, - method_ident: Ident::new(self.name, trait_.span), + method_ident: Ident::new(self.name, span), self_args, nonself_args, fields, }; let mut f = self.combine_substructure.borrow_mut(); let f: &mut CombineSubstructureFunc<'_> = &mut *f; - f(cx, trait_.span, &substructure) + f(cx, span, &substructure) } fn get_ret_ty( @@ -869,9 +885,10 @@ impl<'a> MethodDef<'a> { let mut nonself_args = Vec::new(); let mut arg_tys = Vec::new(); let mut nonstatic = false; + let span = trait_.span; let ast_explicit_self = self.explicit_self.as_ref().map(|self_ptr| { - let (self_expr, explicit_self) = ty::get_explicit_self(cx, trait_.span, self_ptr); + let (self_expr, explicit_self) = ty::get_explicit_self(cx, span, self_ptr); self_args.push(self_expr); nonstatic = true; @@ -880,11 +897,11 @@ impl<'a> MethodDef<'a> { }); for (ty, name) in self.args.iter() { - let ast_ty = ty.to_ty(cx, trait_.span, type_ident, generics); - let ident = Ident::new(*name, trait_.span); + let ast_ty = ty.to_ty(cx, span, type_ident, generics); + let ident = Ident::new(*name, span); arg_tys.push((ident, ast_ty)); - let arg_expr = cx.expr_ident(trait_.span, ident); + let arg_expr = cx.expr_ident(span, ident); match *ty { // for static methods, just treat any Self @@ -893,7 +910,7 @@ impl<'a> MethodDef<'a> { self_args.push(arg_expr); } Ptr(ref ty, _) if matches!(**ty, Self_) && nonstatic => { - self_args.push(cx.expr_deref(trait_.span, arg_expr)) + self_args.push(cx.expr_deref(span, arg_expr)) } _ => { nonself_args.push(arg_expr); @@ -914,33 +931,33 @@ impl<'a> MethodDef<'a> { arg_types: Vec<(Ident, P<ast::Ty>)>, body: P<Expr>, ) -> P<ast::AssocItem> { + let span = trait_.span; // Create the generics that aren't for `Self`. - let fn_generics = self.generics.to_generics(cx, trait_.span, type_ident, generics); + let fn_generics = self.generics.to_generics(cx, span, type_ident, generics); let args = { let self_args = explicit_self.map(|explicit_self| { - let ident = Ident::with_dummy_span(kw::SelfLower).with_span_pos(trait_.span); + let ident = Ident::with_dummy_span(kw::SelfLower).with_span_pos(span); ast::Param::from_self(ast::AttrVec::default(), explicit_self, ident) }); - let nonself_args = - arg_types.into_iter().map(|(name, ty)| cx.param(trait_.span, name, ty)); + let nonself_args = arg_types.into_iter().map(|(name, ty)| cx.param(span, name, ty)); self_args.into_iter().chain(nonself_args).collect() }; let ret_type = self.get_ret_ty(cx, trait_, generics, type_ident); - let method_ident = Ident::new(self.name, trait_.span); + let method_ident = Ident::new(self.name, span); let fn_decl = cx.fn_decl(args, ast::FnRetTy::Ty(ret_type)); let body_block = cx.block_expr(body); - let unsafety = if self.is_unsafe { ast::Unsafe::Yes(trait_.span) } else { ast::Unsafe::No }; + let unsafety = if self.is_unsafe { ast::Unsafe::Yes(span) } else { ast::Unsafe::No }; - let trait_lo_sp = trait_.span.shrink_to_lo(); + let trait_lo_sp = span.shrink_to_lo(); let sig = ast::FnSig { header: ast::FnHeader { unsafety, ext: ast::Extern::None, ..ast::FnHeader::default() }, decl: fn_decl, - span: trait_.span, + span, }; let defaultness = ast::Defaultness::Final; @@ -948,7 +965,7 @@ impl<'a> MethodDef<'a> { P(ast::AssocItem { id: ast::DUMMY_NODE_ID, attrs: self.attributes.clone(), - span: trait_.span, + span, vis: ast::Visibility { span: trait_lo_sp, kind: ast::VisibilityKind::Inherited, @@ -1011,11 +1028,11 @@ impl<'a> MethodDef<'a> { nonself_args: &[P<Expr>], use_temporaries: bool, ) -> P<Expr> { - let mut raw_fields = Vec::new(); // Vec<[fields of self], - // [fields of next Self arg], [etc]> + let mut raw_fields = Vec::new(); // Vec<[fields of self], [fields of next Self arg], [etc]> + let span = trait_.span; let mut patterns = Vec::new(); for i in 0..self_args.len() { - let struct_path = cx.path(trait_.span, vec![type_ident]); + let struct_path = cx.path(span, vec![type_ident]); let (pat, ident_expr) = trait_.create_struct_pattern( cx, struct_path, @@ -1035,7 +1052,7 @@ impl<'a> MethodDef<'a> { let mut other_fields: Vec<vec::IntoIter<_>> = raw_fields.collect(); first_field .map(|(span, opt_id, field, attrs)| FieldInfo { - span, + span: span.with_ctxt(trait_.span.ctxt()), name: opt_id, self_: field, other: other_fields @@ -1049,7 +1066,7 @@ impl<'a> MethodDef<'a> { }) .collect() } else { - cx.span_bug(trait_.span, "no `self` parameter for method in generic `derive`") + cx.span_bug(span, "no `self` parameter for method in generic `derive`") }; // body of the inner most destructuring match @@ -1066,11 +1083,7 @@ impl<'a> MethodDef<'a> { // structs. This is actually right-to-left, but it shouldn't // matter. for (arg_expr, pat) in iter::zip(self_args, patterns) { - body = cx.expr_match( - trait_.span, - arg_expr.clone(), - vec![cx.arm(trait_.span, pat.clone(), body)], - ) + body = cx.expr_match(span, arg_expr.clone(), vec![cx.arm(span, pat.clone(), body)]) } body @@ -1180,7 +1193,7 @@ impl<'a> MethodDef<'a> { mut self_args: Vec<P<Expr>>, nonself_args: &[P<Expr>], ) -> P<Expr> { - let sp = trait_.span; + let span = trait_.span; let variants = &enum_def.variants; let self_arg_names = iter::once("__self".to_string()) @@ -1195,7 +1208,7 @@ impl<'a> MethodDef<'a> { let self_arg_idents = self_arg_names .iter() - .map(|name| Ident::from_str_and_span(name, sp)) + .map(|name| Ident::from_str_and_span(name, span)) .collect::<Vec<Ident>>(); // The `vi_idents` will be bound, solely in the catch-all, to @@ -1205,7 +1218,7 @@ impl<'a> MethodDef<'a> { .iter() .map(|name| { let vi_suffix = format!("{}_vi", &name[..]); - Ident::from_str_and_span(&vi_suffix, trait_.span) + Ident::from_str_and_span(&vi_suffix, span) }) .collect::<Vec<Ident>>(); @@ -1235,7 +1248,7 @@ impl<'a> MethodDef<'a> { self_arg_name, ast::Mutability::Not, ); - (cx.pat(sp, PatKind::Ref(p, ast::Mutability::Not)), idents) + (cx.pat(span, PatKind::Ref(p, ast::Mutability::Not)), idents) }; // A single arm has form (&VariantK, &VariantK, ...) => BodyK @@ -1254,7 +1267,7 @@ impl<'a> MethodDef<'a> { } // Here is the pat = `(&VariantK, &VariantK, ...)` - let single_pat = cx.pat_tuple(sp, subpats); + let single_pat = cx.pat_tuple(span, subpats); // For the BodyK, we need to delegate to our caller, // passing it an EnumMatching to indicate which case @@ -1271,7 +1284,7 @@ impl<'a> MethodDef<'a> { .into_iter() .enumerate() // For each arg field of self, pull out its getter expr ... - .map(|(field_index, (sp, opt_ident, self_getter_expr, attrs))| { + .map(|(field_index, (span, opt_ident, self_getter_expr, attrs))| { // ... but FieldInfo also wants getter expr // for matching other arguments of Self type; // so walk across the *other* self_pats_idents @@ -1294,7 +1307,7 @@ impl<'a> MethodDef<'a> { .collect::<Vec<P<Expr>>>(); FieldInfo { - span: sp, + span, name: opt_ident, self_: self_getter_expr, other: others, @@ -1317,7 +1330,7 @@ impl<'a> MethodDef<'a> { &substructure, ); - cx.arm(sp, single_pat, arm_expr) + cx.arm(span, single_pat, arm_expr) }) .collect(); @@ -1340,12 +1353,12 @@ impl<'a> MethodDef<'a> { // Since we know that all the arguments will match if we reach // the match expression we add the unreachable intrinsics as the // result of the catch all which should help llvm in optimizing it - Some(deriving::call_unreachable(cx, sp)) + Some(deriving::call_unreachable(cx, span)) } _ => None, }; if let Some(arm) = default { - match_arms.push(cx.arm(sp, cx.pat_wild(sp), arm)); + match_arms.push(cx.arm(span, cx.pat_wild(span), arm)); } // We will usually need the catch-all after matching the @@ -1379,23 +1392,23 @@ impl<'a> MethodDef<'a> { // We also build an expression which checks whether all discriminants are equal // discriminant_test = __self0_vi == __self1_vi && __self0_vi == __self2_vi && ... - let mut discriminant_test = cx.expr_bool(sp, true); + let mut discriminant_test = cx.expr_bool(span, true); let mut first_ident = None; for (&ident, self_arg) in iter::zip(&vi_idents, &self_args) { - let self_addr = cx.expr_addr_of(sp, self_arg.clone()); + let self_addr = cx.expr_addr_of(span, self_arg.clone()); let variant_value = - deriving::call_intrinsic(cx, sp, sym::discriminant_value, vec![self_addr]); - let let_stmt = cx.stmt_let(sp, false, ident, variant_value); + deriving::call_intrinsic(cx, span, sym::discriminant_value, vec![self_addr]); + let let_stmt = cx.stmt_let(span, false, ident, variant_value); index_let_stmts.push(let_stmt); match first_ident { Some(first) => { - let first_expr = cx.expr_ident(sp, first); - let id = cx.expr_ident(sp, ident); - let test = cx.expr_binary(sp, BinOpKind::Eq, first_expr, id); + let first_expr = cx.expr_ident(span, first); + let id = cx.expr_ident(span, ident); + let test = cx.expr_binary(span, BinOpKind::Eq, first_expr, id); discriminant_test = - cx.expr_binary(sp, BinOpKind::And, discriminant_test, test) + cx.expr_binary(span, BinOpKind::And, discriminant_test, test) } None => { first_ident = Some(ident); @@ -1417,8 +1430,8 @@ impl<'a> MethodDef<'a> { // them when they are fed as r-values into a tuple // expression; here add a layer of borrowing, turning // `(*self, *__arg_0, ...)` into `(&*self, &*__arg_0, ...)`. - self_args.map_in_place(|self_arg| cx.expr_addr_of(sp, self_arg)); - let match_arg = cx.expr(sp, ast::ExprKind::Tup(self_args)); + self_args.map_in_place(|self_arg| cx.expr_addr_of(span, self_arg)); + let match_arg = cx.expr(span, ast::ExprKind::Tup(self_args)); // Lastly we create an expression which branches on all discriminants being equal // if discriminant_test { @@ -1432,10 +1445,10 @@ impl<'a> MethodDef<'a> { // else { // <delegated expression referring to __self0_vi, et al.> // } - let all_match = cx.expr_match(sp, match_arg, match_arms); - let arm_expr = cx.expr_if(sp, discriminant_test, all_match, Some(arm_expr)); + let all_match = cx.expr_match(span, match_arg, match_arms); + let arm_expr = cx.expr_if(span, discriminant_test, all_match, Some(arm_expr)); index_let_stmts.push(cx.stmt_expr(arm_expr)); - cx.expr_block(cx.block(sp, index_let_stmts)) + cx.expr_block(cx.block(span, index_let_stmts)) } else if variants.is_empty() { // As an additional wrinkle, For a zero-variant enum A, // currently the compiler @@ -1486,16 +1499,16 @@ impl<'a> MethodDef<'a> { // derive Debug on such a type could here generate code // that needs the feature gate enabled.) - deriving::call_unreachable(cx, sp) + deriving::call_unreachable(cx, span) } else { // Final wrinkle: the self_args are expressions that deref // down to desired places, but we cannot actually deref // them when they are fed as r-values into a tuple // expression; here add a layer of borrowing, turning // `(*self, *__arg_0, ...)` into `(&*self, &*__arg_0, ...)`. - self_args.map_in_place(|self_arg| cx.expr_addr_of(sp, self_arg)); - let match_arg = cx.expr(sp, ast::ExprKind::Tup(self_args)); - cx.expr_match(sp, match_arg, match_arms) + self_args.map_in_place(|self_arg| cx.expr_addr_of(span, self_arg)); + let match_arg = cx.expr(span, ast::ExprKind::Tup(self_args)); + cx.expr_match(span, match_arg, match_arms) } } diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index d5132ad5bdf..c8f53ed2c5b 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -17,7 +17,7 @@ use rustc_index::vec::IndexVec; use rustc_macros::HashStable_Generic; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident, Symbol}; -use rustc_span::{def_id::LocalDefId, BytePos, ExpnKind, MacroKind, MultiSpan, Span, DUMMY_SP}; +use rustc_span::{def_id::LocalDefId, BytePos, MultiSpan, Span, DUMMY_SP}; use rustc_target::asm::InlineAsmRegOrRegClass; use rustc_target::spec::abi::Abi; @@ -525,20 +525,21 @@ pub struct GenericParam<'hir> { } impl GenericParam<'hir> { - pub fn bounds_span(&self) -> Option<Span> { - self.bounds.iter().fold(None, |span, bound| { - if let ExpnKind::Macro(MacroKind::Derive, _) = - bound.span().ctxt().outer_expn_data().kind - { - // We ignore bounds that come from exclusively from a `#[derive(_)]`, because we - // can't really point at them, and we sometimes use this method to get a span - // appropriate for suggestions. - span - } else { - let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span()); - Some(span) - } - }) + pub fn bounds_span_for_suggestions(&self) -> Option<Span> { + self.bounds + .iter() + .fold(None, |span: Option<Span>, bound| { + // We include bounds that come from a `#[derive(_)]` but point at the user's code, + // as we use this method to get a span appropriate for suggestions. + // FIXME: a more "principled" approach should be taken here. + if !bound.span().can_be_used_for_suggestions() { + None + } else { + let span = span.map(|s| s.to(bound.span())).unwrap_or_else(|| bound.span()); + Some(span) + } + }) + .map(|sp| sp.shrink_to_hi()) } } diff --git a/compiler/rustc_middle/src/ty/diagnostics.rs b/compiler/rustc_middle/src/ty/diagnostics.rs index 1b32c8a6698..8803370251b 100644 --- a/compiler/rustc_middle/src/ty/diagnostics.rs +++ b/compiler/rustc_middle/src/ty/diagnostics.rs @@ -270,7 +270,7 @@ pub fn suggest_constraining_type_param( // `where` clause instead of `trait Base<T: Copy = String>: Super<T>`. && !matches!(param.kind, hir::GenericParamKind::Type { default: Some(_), .. }) { - if let Some(bounds_span) = param.bounds_span() { + if let Some(span) = param.bounds_span_for_suggestions() { // If user has provided some bounds, suggest restricting them: // // fn foo<T: Foo>(t: T) { ... } @@ -284,7 +284,7 @@ pub fn suggest_constraining_type_param( // -- // | // replace with: `T: Bar +` - suggest_restrict(bounds_span.shrink_to_hi()); + suggest_restrict(span); } else { // If user hasn't provided any bounds, suggest adding a new one: // diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index 4e071d69e36..72ba3f7b980 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -1735,7 +1735,7 @@ impl<'a: 'ast, 'ast> LateResolutionVisitor<'a, '_, 'ast> { (generics.span, format!("<{}>", ident)) }; // Do not suggest if this is coming from macro expansion. - if !span.from_expansion() { + if span.can_be_used_for_suggestions() { return Some(( span.shrink_to_hi(), msg, @@ -1825,7 +1825,7 @@ impl<'tcx> LifetimeContext<'_, 'tcx> { continue; } suggested_spans.push(span); - if !span.from_expansion() { + if span.can_be_used_for_suggestions() { err.span_suggestion( span, &format!("consider introducing lifetime `{}` here", lifetime_ref), diff --git a/compiler/rustc_span/src/lib.rs b/compiler/rustc_span/src/lib.rs index 66c01140abc..98478cf5dec 100644 --- a/compiler/rustc_span/src/lib.rs +++ b/compiler/rustc_span/src/lib.rs @@ -551,6 +551,16 @@ impl Span { matches!(self.ctxt().outer_expn_data().kind, ExpnKind::Macro(MacroKind::Derive, _)) } + /// Gate suggestions that would not be appropriate in a context the user didn't write. + pub fn can_be_used_for_suggestions(self) -> bool { + !self.from_expansion() + // FIXME: If this span comes from a `derive` macro but it points at code the user wrote, + // the callsite span and the span will be pointing at different places. It also means that + // we can safely provide suggestions on this span. + || (matches!(self.ctxt().outer_expn_data().kind, ExpnKind::Macro(MacroKind::Derive, _)) + && self.parent_callsite().map(|p| (p.lo(), p.hi())) != Some((self.lo(), self.hi()))) + } + #[inline] pub fn with_root_ctxt(lo: BytePos, hi: BytePos) -> Span { Span::new(lo, hi, SyntaxContext::root(), None) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index af0a39b239f..4c09aa1183f 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -262,8 +262,8 @@ fn suggest_restriction( match generics .params .iter() - .map(|p| p.bounds_span().unwrap_or(p.span)) - .filter(|&span| generics.span.contains(span) && span.desugaring_kind().is_none()) + .map(|p| p.bounds_span_for_suggestions().unwrap_or(p.span.shrink_to_hi())) + .filter(|&span| generics.span.contains(span) && span.can_be_used_for_suggestions()) .max_by_key(|span| span.hi()) { // `fn foo(t: impl Trait)` @@ -271,7 +271,7 @@ fn suggest_restriction( None => (generics.span, format!("<{}>", type_param)), // `fn foo<A>(t: impl Trait)` // ^^^ suggest `<A, T: Trait>` here - Some(span) => (span.shrink_to_hi(), format!(", {}", type_param)), + Some(span) => (span, format!(", {}", type_param)), }, // `fn foo(t: impl Trait)` // ^ suggest `where <T as Trait>::A: Bound` diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index bc41c230b11..4d86755b26c 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -177,11 +177,9 @@ crate fn placeholder_type_error( sugg.push((arg.span, (*type_name).to_string())); } else { let last = generics.iter().last().unwrap(); - sugg.push(( - // Account for bounds, we want `fn foo<T: E, K>(_: K)` not `fn foo<T, K: E>(_: K)`. - last.bounds_span().unwrap_or(last.span).shrink_to_hi(), - format!(", {}", type_name), - )); + // Account for bounds, we want `fn foo<T: E, K>(_: K)` not `fn foo<T, K: E>(_: K)`. + let span = last.bounds_span_for_suggestions().unwrap_or(last.span.shrink_to_hi()); + sugg.push((span, format!(", {}", type_name))); } let mut err = bad_placeholder_type(tcx, placeholder_types, kind); diff --git a/src/test/ui/issues/issue-38821.stderr b/src/test/ui/issues/issue-38821.stderr index 3a8b69224d3..cdf1f0dfc53 100644 --- a/src/test/ui/issues/issue-38821.stderr +++ b/src/test/ui/issues/issue-38821.stderr @@ -12,8 +12,8 @@ LL | impl<T: NotNull> IntoNullable for T { = note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider further restricting the associated type | -LL | Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull - | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +LL | Expr: Expression<SqlType=<Col::SqlType as IntoNullable>::Nullable>, <Col as Expression>::SqlType: NotNull, + | +++++++++++++++++++++++++++++++++++++++ error: aborting due to previous error From b0c3968615d8728fb0476bd81fa58903aca5d191 Mon Sep 17 00:00:00 2001 From: Esteban Kuber <esteban@kuber.com.ar> Date: Fri, 3 Dec 2021 18:45:15 +0000 Subject: [PATCH 6/7] review comment --- compiler/rustc_hir/src/hir.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index c8f53ed2c5b..23c4f72bae7 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -531,7 +531,6 @@ impl GenericParam<'hir> { .fold(None, |span: Option<Span>, bound| { // We include bounds that come from a `#[derive(_)]` but point at the user's code, // as we use this method to get a span appropriate for suggestions. - // FIXME: a more "principled" approach should be taken here. if !bound.span().can_be_used_for_suggestions() { None } else { From aef59e4fb87fe8c79d3c5d6e7164c02c84fe2b8b Mon Sep 17 00:00:00 2001 From: kit <kit@hastur.io> Date: Sun, 11 Jul 2021 15:43:30 +1000 Subject: [PATCH 7/7] Add a `try_reduce` method to the Iterator trait --- library/core/src/iter/traits/iterator.rs | 80 ++++++++++++++++++++++ library/core/tests/iter/traits/iterator.rs | 28 ++++++++ library/core/tests/lib.rs | 1 + 3 files changed, 109 insertions(+) diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index 88e7623eba1..267fa406798 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -2216,6 +2216,86 @@ pub trait Iterator { Some(self.fold(first, f)) } + /// Reduces the elements to a single one by repeatedly applying a reducing operation. If the + /// closure returns a failure, the failure is propagated back to the caller immediately. + /// + /// The return type of this method depends on the return type of the closure. If the closure + /// returns `Result<Self::Item, E>`, then this function will return `Result<Option<Self::Item>, + /// E>`. If the closure returns `Option<Self::Item>`, then this function will return + /// `Option<Option<Self::Item>>`. + /// + /// When called on an empty iterator, this function will return either `Some(None)` or + /// `Ok(None)` depending on the type of the provided closure. + /// + /// For iterators with at least one element, this is essentially the same as calling + /// [`try_fold()`] with the first element of the iterator as the initial accumulator value. + /// + /// [`try_fold()`]: Iterator::try_fold + /// + /// # Examples + /// + /// Safely calculate the sum of a series of numbers: + /// + /// ``` + /// #![feature(iterator_try_reduce)] + /// + /// let numbers: Vec<usize> = vec![10, 20, 5, 23, 0]; + /// let sum = numbers.into_iter().try_reduce(|x, y| x.checked_add(y)); + /// assert_eq!(sum, Some(Some(58))); + /// ``` + /// + /// Determine when a reduction short circuited: + /// + /// ``` + /// #![feature(iterator_try_reduce)] + /// + /// let numbers = vec![1, 2, 3, usize::MAX, 4, 5]; + /// let sum = numbers.into_iter().try_reduce(|x, y| x.checked_add(y)); + /// assert_eq!(sum, None); + /// ``` + /// + /// Determine when a reduction was not performed because there are no elements: + /// + /// ``` + /// #![feature(iterator_try_reduce)] + /// + /// let numbers: Vec<usize> = Vec::new(); + /// let sum = numbers.into_iter().try_reduce(|x, y| x.checked_add(y)); + /// assert_eq!(sum, Some(None)); + /// ``` + /// + /// Use a [`Result`] instead of an [`Option`]: + /// + /// ``` + /// #![feature(iterator_try_reduce)] + /// + /// let numbers = vec!["1", "2", "3", "4", "5"]; + /// let max: Result<Option<_>, <usize as std::str::FromStr>::Err> = + /// numbers.into_iter().try_reduce(|x, y| { + /// if x.parse::<usize>()? > y.parse::<usize>()? { Ok(x) } else { Ok(y) } + /// }); + /// assert_eq!(max, Ok(Some("5"))); + /// ``` + #[inline] + #[unstable(feature = "iterator_try_reduce", reason = "new API", issue = "87053")] + fn try_reduce<F, R>(&mut self, f: F) -> ChangeOutputType<R, Option<R::Output>> + where + Self: Sized, + F: FnMut(Self::Item, Self::Item) -> R, + R: Try<Output = Self::Item>, + R::Residual: Residual<Option<Self::Item>>, + { + let first = match self.next() { + Some(i) => i, + None => return Try::from_output(None), + }; + + match self.try_fold(first, f).branch() { + ControlFlow::Break(r) => FromResidual::from_residual(r), + ControlFlow::Continue(i) => Try::from_output(Some(i)), + } + } + /// Tests if every element of the iterator matches a predicate. /// /// `all()` takes a closure that returns `true` or `false`. It applies diff --git a/library/core/tests/iter/traits/iterator.rs b/library/core/tests/iter/traits/iterator.rs index 422e389e380..d38bca1e3b3 100644 --- a/library/core/tests/iter/traits/iterator.rs +++ b/library/core/tests/iter/traits/iterator.rs @@ -454,6 +454,34 @@ fn test_find_map() { } } +#[test] +fn test_try_reduce() { + let v: Vec<usize> = vec![1, 2, 3, 4, 5]; + let sum = v.into_iter().try_reduce(|x, y| x.checked_add(y)); + assert_eq!(sum, Some(Some(15))); + + let v: Vec<usize> = vec![1, 2, 3, 4, 5, usize::MAX]; + let sum = v.into_iter().try_reduce(|x, y| x.checked_add(y)); + assert_eq!(sum, None); + + let v: Vec<usize> = Vec::new(); + let sum = v.into_iter().try_reduce(|x, y| x.checked_add(y)); + assert_eq!(sum, Some(None)); + + let v = vec!["1", "2", "3", "4", "5"]; + let max = v.into_iter().try_reduce(|x, y| { + if x.parse::<usize>().ok()? > y.parse::<usize>().ok()? { Some(x) } else { Some(y) } + }); + assert_eq!(max, Some(Some("5"))); + + let v = vec!["1", "2", "3", "4", "5"]; + let max: Result<Option<_>, <usize as std::str::FromStr>::Err> = + v.into_iter().try_reduce(|x, y| { + if x.parse::<usize>()? > y.parse::<usize>()? { Ok(x) } else { Ok(y) } + }); + assert_eq!(max, Ok(Some("5"))); +} + #[test] fn test_iterator_len() { let v: &[_] = &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; diff --git a/library/core/tests/lib.rs b/library/core/tests/lib.rs index d49c2552cfd..9ab98ba8886 100644 --- a/library/core/tests/lib.rs +++ b/library/core/tests/lib.rs @@ -56,6 +56,7 @@ #![feature(iter_intersperse)] #![feature(iter_is_partitioned)] #![feature(iter_order_by)] +#![feature(iterator_try_reduce)] #![feature(const_mut_refs)] #![feature(const_pin)] #![feature(const_slice_from_raw_parts)]