Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not consider repeated lifetime params for elision. #103450

Merged
merged 2 commits into from
Oct 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 69 additions & 34 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -565,7 +566,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<FxIndexMap<LifetimeRes, LifetimeElisionCandidate>>,
lifetime_elision_candidates: Option<Vec<(LifetimeRes, LifetimeElisionCandidate)>>,

/// The trait that the current context can refer to.
current_trait_ref: Option<(Module<'a>, TraitRef)>,
Expand Down Expand Up @@ -1799,7 +1800,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 { .. } => {}
Expand Down Expand Up @@ -1852,12 +1853,25 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
has_self: bool,
inputs: impl Iterator<Item = (Option<&'ast Pat>, &'ast Ty)>,
) -> Result<LifetimeRes, (Vec<MissingLifetime>, Vec<ElisionFnParameter>)> {
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() {
Expand All @@ -1867,61 +1881,82 @@ 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 {
Some(ident)
} 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 all_candidates.len() == 1 {
Ok(*all_candidates.first().unwrap().0)
} 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.
Expand Down Expand Up @@ -2391,7 +2426,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);
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,10 @@ fn k<'a, T: WithLifetime<'a>>(_x: T::Output) -> &isize {
panic!()
}

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() {}
Original file line number Diff line number Diff line change
Expand Up @@ -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`.