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 1 commit
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
12 changes: 8 additions & 4 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ 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>,
in_scope_lifetimes: Vec<ParamName>,

current_module: NodeId,

Expand Down Expand Up @@ -865,7 +865,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 +899,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 +2267,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
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,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),
_ => None,
});
self.in_scope_lifetimes.extend(lt_def_names);
Expand Down
16 changes: 16 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,16 @@
// Check that `async fn` inside of an impl with `'_`
// in the header compiles correctly.
//
// Regression test for #63500.
//
// check-pass

#![feature(async_await)]

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

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

fn main() { }