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

use ParamName to track in-scope lifetimes instead of Ident #63501

Merged
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
15 changes: 11 additions & 4 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ pub struct LoweringContext<'a> {
/// When `is_collectin_in_band_lifetimes` is true, each lifetime is checked
/// against this list to see if it is already in-scope, or if a definition
/// needs to be created for it.
in_scope_lifetimes: Vec<Ident>,
///
/// We always store a `modern()` version of the param-name in this
/// vector.
in_scope_lifetimes: Vec<ParamName>,

current_module: NodeId,

Expand Down Expand Up @@ -865,7 +868,7 @@ impl<'a> LoweringContext<'a> {
return;
}

if self.in_scope_lifetimes.contains(&ident.modern()) {
if self.in_scope_lifetimes.contains(&ParamName::Plain(ident.modern())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this check is correct anymore-- it's failing when it should succeed, causing lifetimes to be introduced multiple times (see the travis error). It should instead check if there is an in scope lifetime that uses the same identifier, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Is it the ident.modern() that's causing the problem, you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I have a fix now

return;
}

Expand Down Expand Up @@ -899,7 +902,7 @@ impl<'a> LoweringContext<'a> {
{
let old_len = self.in_scope_lifetimes.len();
let lt_def_names = params.iter().filter_map(|param| match param.kind {
GenericParamKind::Lifetime { .. } => Some(param.ident.modern()),
GenericParamKind::Lifetime { .. } => Some(ParamName::Plain(param.ident.modern())),
_ => None,
});
self.in_scope_lifetimes.extend(lt_def_names);
Expand Down Expand Up @@ -2267,10 +2270,14 @@ impl<'a> LoweringContext<'a> {
let lifetime_params: Vec<(Span, ParamName)> =
this.in_scope_lifetimes
.iter().cloned()
.map(|ident| (ident.span, ParamName::Plain(ident)))
.map(|name| (name.ident().span, name))
.chain(this.lifetimes_to_define.iter().cloned())
.collect();

debug!("lower_async_fn_ret_ty: in_scope_lifetimes={:#?}", this.in_scope_lifetimes);
debug!("lower_async_fn_ret_ty: lifetimes_to_define={:#?}", this.lifetimes_to_define);
debug!("lower_async_fn_ret_ty: lifetime_params={:#?}", lifetime_params);

let generic_params =
lifetime_params
.iter().cloned()
Expand Down
34 changes: 29 additions & 5 deletions src/librustc/hir/lowering/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,12 @@ impl<'tcx, 'interner> Visitor<'tcx> for ItemLowerer<'tcx, 'interner> {
fn visit_item(&mut self, item: &'tcx Item) {
let mut item_hir_id = None;
self.lctx.with_hir_id_owner(item.id, |lctx| {
if let Some(hir_item) = lctx.lower_item(item) {
item_hir_id = Some(hir_item.hir_id);
lctx.insert_item(hir_item);
}
lctx.without_in_scope_lifetime_defs(|lctx| {
if let Some(hir_item) = lctx.lower_item(item) {
item_hir_id = Some(hir_item.hir_id);
lctx.insert_item(hir_item);
}
})
});

if let Some(hir_id) = item_hir_id {
Expand Down Expand Up @@ -123,7 +125,7 @@ impl LoweringContext<'_> {
_ => &[],
};
let lt_def_names = parent_generics.iter().filter_map(|param| match param.kind {
hir::GenericParamKind::Lifetime { .. } => Some(param.name.ident().modern()),
hir::GenericParamKind::Lifetime { .. } => Some(param.name.modern()),
_ => None,
});
self.in_scope_lifetimes.extend(lt_def_names);
Expand All @@ -134,6 +136,28 @@ impl LoweringContext<'_> {
res
}

// Clears (and restores) the `in_scope_lifetimes` field. Used when
// visiting nested items, which never inherit in-scope lifetimes
// from their surrounding environment.
fn without_in_scope_lifetime_defs<T>(
&mut self,
f: impl FnOnce(&mut LoweringContext<'_>) -> T,
) -> T {
let old_in_scope_lifetimes = std::mem::replace(&mut self.in_scope_lifetimes, vec![]);

// this vector is only used when walking over impl headers,
// input types, and the like, and should not be non-empty in
// between items
assert!(self.lifetimes_to_define.is_empty());

let res = f(self);

assert!(self.in_scope_lifetimes.is_empty());
self.in_scope_lifetimes = old_in_scope_lifetimes;

res
}

pub(super) fn lower_mod(&mut self, m: &Mod) -> hir::Mod {
hir::Mod {
inner: m.inner,
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/async-await/async-fn-elided-impl-lifetime-parameter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Check that `async fn` inside of an impl with `'_`
// in the header compiles correctly.
//
// Regression test for #63500.
//
// check-pass
// edition:2018

#![feature(async_await)]

struct Foo<'a>(&'a u8);

impl Foo<'_> {
async fn bar() {}
}

fn main() { }
17 changes: 17 additions & 0 deletions src/test/ui/async-await/nested-in-impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Test that async fn works when nested inside of
// impls with lifetime parameters.
//
// check-pass
// edition:2018

#![feature(async_await)]

struct Foo<'a>(&'a ());

impl<'a> Foo<'a> {
fn test() {
async fn test() {}
}
}

fn main() { }
20 changes: 20 additions & 0 deletions src/test/ui/in-band-lifetimes/nested-items.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Test that the `'a` from the impl doesn't
// prevent us from creating a `'a` parameter
// on the `blah` function.
//
// check-pass

#![feature(in_band_lifetimes)]

struct Foo<'a> {
x: &'a u32

}

impl Foo<'a> {
fn method(&self) {
fn blah(f: Foo<'a>) { }
}
}

fn main() { }