From 47704bbcc063c2a8f3e88b06cf2f54f6b64b5ae7 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 23 Oct 2022 19:35:47 +0000 Subject: [PATCH 1/2] Do not consider repeated lifetime params for elision. --- compiler/rustc_resolve/src/late.rs | 10 +++++----- ...ision-return-type-requires-explicit-lifetime.rs | 3 +++ ...n-return-type-requires-explicit-lifetime.stderr | 14 +++++++++++++- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index ba3d8f64bbc76..40e71d4fc751d 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -565,7 +565,7 @@ struct LateResolutionVisitor<'a, 'b, 'ast> { /// They will be used to determine the correct lifetime for the fn return type. /// The `LifetimeElisionCandidate` is used for diagnostics, to suggest introducing named /// lifetimes. - lifetime_elision_candidates: Option>, + lifetime_elision_candidates: Option>, /// The trait that the current context can refer to. current_trait_ref: Option<(Module<'a>, TraitRef)>, @@ -1799,7 +1799,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { match res { LifetimeRes::Param { .. } | LifetimeRes::Fresh { .. } | LifetimeRes::Static => { if let Some(ref mut candidates) = self.lifetime_elision_candidates { - candidates.insert(res, candidate); + candidates.push((res, candidate)); } } LifetimeRes::Infer | LifetimeRes::Error | LifetimeRes::ElidedAnchor { .. } => {} @@ -1910,8 +1910,8 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // We do not have a `self` candidate, look at the full list. let all_candidates = all_candidates.unwrap(); - if all_candidates.len() == 1 { - Ok(*all_candidates.first().unwrap().0) + if let [(res, _)] = &all_candidates[..] { + Ok(*res) } else { let all_candidates = all_candidates .into_iter() @@ -2391,7 +2391,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { // Do not account for the parameters we just bound for function lifetime elision. if let Some(ref mut candidates) = self.lifetime_elision_candidates { for (_, res) in function_lifetime_rib.bindings.values() { - candidates.remove(res); + candidates.retain(|(r, _)| r != res); } } diff --git a/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.rs b/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.rs index 7a2eba518fefa..ba769a4bcc0a9 100644 --- a/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.rs +++ b/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.rs @@ -42,4 +42,7 @@ fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { panic!() } +fn l<'a>(_: &'a str, _: &'a str) -> &str { "" } +//~^ ERROR missing lifetime specifier + fn main() {} diff --git a/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr b/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr index d07754879559a..5eee953ef189f 100644 --- a/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr +++ b/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.stderr @@ -70,6 +70,18 @@ help: consider using the `'a` lifetime LL | fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &'a isize { | ++ -error: aborting due to 6 previous errors +error[E0106]: missing lifetime specifier + --> $DIR/lifetime-elision-return-type-requires-explicit-lifetime.rs:45:37 + | +LL | fn l<'a>(_: &'a str, _: &'a str) -> &str { "" } + | ------- ------- ^ expected named lifetime parameter + | + = help: this function's return type contains a borrowed value with an elided lifetime, but the lifetime cannot be derived from the arguments +help: consider using the `'a` lifetime + | +LL | fn l<'a>(_: &'a str, _: &'a str) -> &'a str { "" } + | ++ + +error: aborting due to 7 previous errors For more information about this error, try `rustc --explain E0106`. From cb1e7d96767525fde3965f86bace8a5acf0dc643 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 27 Oct 2022 17:24:38 +0000 Subject: [PATCH 2/2] Only ban duplication across parameters. --- compiler/rustc_resolve/src/late.rs | 97 +++++++++++++------ compiler/rustc_resolve/src/lib.rs | 1 + ...-return-type-requires-explicit-lifetime.rs | 3 + 3 files changed, 70 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index 40e71d4fc751d..17a9553263147 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -30,6 +30,7 @@ use rustc_span::{BytePos, Span}; use smallvec::{smallvec, SmallVec}; use rustc_span::source_map::{respan, Spanned}; +use std::assert_matches::debug_assert_matches; use std::collections::{hash_map::Entry, BTreeSet}; use std::mem::{replace, take}; @@ -1852,12 +1853,25 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { has_self: bool, inputs: impl Iterator, &'ast Ty)>, ) -> Result, Vec)> { - let outer_candidates = - replace(&mut self.lifetime_elision_candidates, Some(Default::default())); + enum Elision { + /// We have not found any candidate. + None, + /// We have a candidate bound to `self`. + Self_(LifetimeRes), + /// We have a candidate bound to a parameter. + Param(LifetimeRes), + /// We failed elision. + Err, + } - let mut elision_lifetime = None; - let mut lifetime_count = 0; + // Save elision state to reinstate it later. + let outer_candidates = self.lifetime_elision_candidates.take(); + + // Result of elision. + let mut elision_lifetime = Elision::None; + // Information for diagnostics. let mut parameter_info = Vec::new(); + let mut all_candidates = Vec::new(); let mut bindings = smallvec![(PatBoundCtx::Product, Default::default())]; for (index, (pat, ty)) in inputs.enumerate() { @@ -1867,12 +1881,17 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { this.resolve_pattern(pat, PatternSource::FnParam, &mut bindings); } }); + + // Record elision candidates only for this parameter. + debug_assert_matches!(self.lifetime_elision_candidates, None); + self.lifetime_elision_candidates = Some(Default::default()); self.visit_ty(ty); + let local_candidates = self.lifetime_elision_candidates.take(); - if let Some(ref candidates) = self.lifetime_elision_candidates { - let new_count = candidates.len(); - let local_count = new_count - lifetime_count; - if local_count != 0 { + if let Some(candidates) = local_candidates { + let distinct: FxHashSet<_> = candidates.iter().map(|(res, _)| *res).collect(); + let lifetime_count = distinct.len(); + if lifetime_count != 0 { parameter_info.push(ElisionFnParameter { index, ident: if let Some(pat) = pat && let PatKind::Ident(_, ident, _) = pat.kind { @@ -1880,48 +1899,64 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { } else { None }, - lifetime_count: local_count, + lifetime_count, span: ty.span, }); + all_candidates.extend(candidates.into_iter().filter_map(|(_, candidate)| { + match candidate { + LifetimeElisionCandidate::Ignore | LifetimeElisionCandidate::Named => { + None + } + LifetimeElisionCandidate::Missing(missing) => Some(missing), + } + })); + } + let mut distinct_iter = distinct.into_iter(); + if let Some(res) = distinct_iter.next() { + match elision_lifetime { + // We are the first parameter to bind lifetimes. + Elision::None => { + if distinct_iter.next().is_none() { + // We have a single lifetime => success. + elision_lifetime = Elision::Param(res) + } else { + // We have have multiple lifetimes => error. + elision_lifetime = Elision::Err; + } + } + // We have 2 parameters that bind lifetimes => error. + Elision::Param(_) => elision_lifetime = Elision::Err, + // `self` elision takes precedence over everything else. + Elision::Self_(_) | Elision::Err => {} + } } - lifetime_count = new_count; } // Handle `self` specially. if index == 0 && has_self { let self_lifetime = self.find_lifetime_for_self(ty); if let Set1::One(lifetime) = self_lifetime { - elision_lifetime = Some(lifetime); - self.lifetime_elision_candidates = None; + // We found `self` elision. + elision_lifetime = Elision::Self_(lifetime); } else { - self.lifetime_elision_candidates = Some(Default::default()); - lifetime_count = 0; + // We do not have `self` elision: disregard the `Elision::Param` that we may + // have found. + elision_lifetime = Elision::None; } } debug!("(resolving function / closure) recorded parameter"); } - let all_candidates = replace(&mut self.lifetime_elision_candidates, outer_candidates); - debug!(?all_candidates); + // Reinstate elision state. + debug_assert_matches!(self.lifetime_elision_candidates, None); + self.lifetime_elision_candidates = outer_candidates; - if let Some(res) = elision_lifetime { + if let Elision::Param(res) | Elision::Self_(res) = elision_lifetime { return Ok(res); } - // We do not have a `self` candidate, look at the full list. - let all_candidates = all_candidates.unwrap(); - if let [(res, _)] = &all_candidates[..] { - Ok(*res) - } else { - let all_candidates = all_candidates - .into_iter() - .filter_map(|(_, candidate)| match candidate { - LifetimeElisionCandidate::Ignore | LifetimeElisionCandidate::Named => None, - LifetimeElisionCandidate::Missing(missing) => Some(missing), - }) - .collect(); - Err((all_candidates, parameter_info)) - } + // We do not have a candidate. + Err((all_candidates, parameter_info)) } /// List all the lifetimes that appear in the provided type. diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 1c1976af5054c..a0f05878283e6 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -7,6 +7,7 @@ //! Type-relative name resolution (methods, fields, associated items) happens in `rustc_hir_analysis`. #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] +#![feature(assert_matches)] #![feature(box_patterns)] #![feature(drain_filter)] #![feature(if_let_guard)] diff --git a/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.rs b/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.rs index ba769a4bcc0a9..d0a8fe795efd0 100644 --- a/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.rs +++ b/src/test/ui/lifetimes/lifetime-elision-return-type-requires-explicit-lifetime.rs @@ -45,4 +45,7 @@ fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize { fn l<'a>(_: &'a str, _: &'a str) -> &str { "" } //~^ ERROR missing lifetime specifier +// This is ok because both `'a` are for the same parameter. +fn m<'a>(_: &'a Foo<'a>) -> &str { "" } + fn main() {}