Skip to content

Commit

Permalink
Rollup merge of #63376 - nikomatsakis:async-await-issue-62517, r=cram…
Browse files Browse the repository at this point in the history
…ertj

use different lifetime name for object-lifetime-default elision

Introduce a distinct value for `LifetimeName` to use when this is a object-lifetime-default elision. This allows us to avoid creating incorrect lifetime parameters for the opaque types that result. We really need to overhaul this setup at some point! It's getting increasingly byzantine. But this seems like a relatively... surgical fix.

r? @cramertj

Fixes #62517
  • Loading branch information
Centril authored Aug 19, 2019
2 parents 4486c02 + 7ee1af5 commit bece117
Show file tree
Hide file tree
Showing 21 changed files with 429 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ pub fn walk_lifetime<'v, V: Visitor<'v>>(visitor: &mut V, lifetime: &'v Lifetime
LifetimeName::Static |
LifetimeName::Error |
LifetimeName::Implicit |
LifetimeName::ImplicitObjectLifetimeDefault |
LifetimeName::Underscore => {}
}
}
Expand Down
61 changes: 58 additions & 3 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ enum ParenthesizedGenericArgs {
/// `resolve_lifetime` module. Often we "fallthrough" to that code by generating
/// an "elided" or "underscore" lifetime name. In the future, we probably want to move
/// everything into HIR lowering.
#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug)]
enum AnonymousLifetimeMode {
/// For **Modern** cases, create a new anonymous region parameter
/// and reference that.
Expand Down Expand Up @@ -715,10 +715,16 @@ impl<'a> LoweringContext<'a> {
anonymous_lifetime_mode: AnonymousLifetimeMode,
op: impl FnOnce(&mut Self) -> R,
) -> R {
debug!(
"with_anonymous_lifetime_mode(anonymous_lifetime_mode={:?})",
anonymous_lifetime_mode,
);
let old_anonymous_lifetime_mode = self.anonymous_lifetime_mode;
self.anonymous_lifetime_mode = anonymous_lifetime_mode;
let result = op(self);
self.anonymous_lifetime_mode = old_anonymous_lifetime_mode;
debug!("with_anonymous_lifetime_mode: restoring anonymous_lifetime_mode={:?}",
old_anonymous_lifetime_mode);
result
}

Expand Down Expand Up @@ -1355,6 +1361,13 @@ impl<'a> LoweringContext<'a> {
opaque_ty_node_id: NodeId,
lower_bounds: impl FnOnce(&mut LoweringContext<'_>) -> hir::GenericBounds,
) -> hir::TyKind {
debug!(
"lower_opaque_impl_trait(fn_def_id={:?}, opaque_ty_node_id={:?}, span={:?})",
fn_def_id,
opaque_ty_node_id,
span,
);

// Make sure we know that some funky desugaring has been going on here.
// This is a first: there is code in other places like for loop
// desugaring that explicitly states that we don't want to track that.
Expand Down Expand Up @@ -1382,6 +1395,14 @@ impl<'a> LoweringContext<'a> {
&hir_bounds,
);

debug!(
"lower_opaque_impl_trait: lifetimes={:#?}", lifetimes,
);

debug!(
"lower_opaque_impl_trait: lifetime_defs={:#?}", lifetime_defs,
);

self.with_hir_id_owner(opaque_ty_node_id, |lctx| {
let opaque_ty_item = hir::OpaqueTy {
generics: hir::Generics {
Expand All @@ -1397,7 +1418,7 @@ impl<'a> LoweringContext<'a> {
origin: hir::OpaqueTyOrigin::FnReturn,
};

trace!("exist ty from impl trait def-index: {:#?}", opaque_ty_def_index);
trace!("lower_opaque_impl_trait: {:#?}", opaque_ty_def_index);
let opaque_ty_id = lctx.generate_opaque_type(
opaque_ty_node_id,
opaque_ty_item,
Expand Down Expand Up @@ -1445,6 +1466,13 @@ impl<'a> LoweringContext<'a> {
parent_index: DefIndex,
bounds: &hir::GenericBounds,
) -> (HirVec<hir::GenericArg>, HirVec<hir::GenericParam>) {
debug!(
"lifetimes_from_impl_trait_bounds(opaque_ty_id={:?}, \
parent_index={:?}, \
bounds={:#?})",
opaque_ty_id, parent_index, bounds,
);

// This visitor walks over `impl Trait` bounds and creates defs for all lifetimes that
// appear in the bounds, excluding lifetimes that are created within the bounds.
// E.g., `'a`, `'b`, but not `'c` in `impl for<'c> SomeTrait<'a, 'b, 'c>`.
Expand Down Expand Up @@ -1532,6 +1560,11 @@ impl<'a> LoweringContext<'a> {
}
}
hir::LifetimeName::Param(_) => lifetime.name,

// Refers to some other lifetime that is "in
// scope" within the type.
hir::LifetimeName::ImplicitObjectLifetimeDefault => return,

hir::LifetimeName::Error | hir::LifetimeName::Static => return,
};

Expand Down Expand Up @@ -2182,6 +2215,14 @@ impl<'a> LoweringContext<'a> {
fn_def_id: DefId,
opaque_ty_node_id: NodeId,
) -> hir::FunctionRetTy {
debug!(
"lower_async_fn_ret_ty(\
output={:?}, \
fn_def_id={:?}, \
opaque_ty_node_id={:?})",
output, fn_def_id, opaque_ty_node_id,
);

let span = output.span();

let opaque_ty_span = self.mark_span_with_reason(
Expand Down Expand Up @@ -2264,6 +2305,8 @@ impl<'a> LoweringContext<'a> {
),
);

debug!("lower_async_fn_ret_ty: future_bound={:#?}", future_bound);

// Calculate all the lifetimes that should be captured
// by the opaque type. This should include all in-scope
// lifetime parameters, including those defined in-band.
Expand Down Expand Up @@ -2512,6 +2555,12 @@ impl<'a> LoweringContext<'a> {
hir::LifetimeName::Implicit
| hir::LifetimeName::Underscore
| hir::LifetimeName::Static => hir::ParamName::Plain(lt.name.ident()),
hir::LifetimeName::ImplicitObjectLifetimeDefault => {
span_bug!(
param.ident.span,
"object-lifetime-default should not occur here",
);
}
hir::LifetimeName::Error => ParamName::Error,
};

Expand Down Expand Up @@ -3255,7 +3304,13 @@ impl<'a> LoweringContext<'a> {
AnonymousLifetimeMode::PassThrough => {}
}

self.new_implicit_lifetime(span)
let r = hir::Lifetime {
hir_id: self.next_id(),
span,
name: hir::LifetimeName::ImplicitObjectLifetimeDefault,
};
debug!("elided_dyn_bound: r={:?}", r);
r
}

fn new_implicit_lifetime(&mut self, span: Span) -> hir::Lifetime {
Expand Down
21 changes: 19 additions & 2 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,19 @@ pub enum LifetimeName {
/// User wrote nothing (e.g., the lifetime in `&u32`).
Implicit,

/// Implicit lifetime in a context like `dyn Foo`. This is
/// distinguished from implicit lifetimes elsewhere because the
/// lifetime that they default to must appear elsewhere within the
/// enclosing type. This means that, in an `impl Trait` context, we
/// don't have to create a parameter for them. That is, `impl
/// Trait<Item = &u32>` expands to an opaque type like `type
/// Foo<'a> = impl Trait<Item = &'a u32>`, but `impl Trait<item =
/// dyn Bar>` expands to `type Foo = impl Trait<Item = dyn Bar +
/// 'static>`. The latter uses `ImplicitObjectLifetimeDefault` so
/// that surrounding code knows not to create a lifetime
/// parameter.
ImplicitObjectLifetimeDefault,

/// Indicates an error during lowering (usually `'_` in wrong place)
/// that was already reported.
Error,
Expand All @@ -235,7 +248,9 @@ pub enum LifetimeName {
impl LifetimeName {
pub fn ident(&self) -> Ident {
match *self {
LifetimeName::Implicit | LifetimeName::Error => Ident::invalid(),
LifetimeName::ImplicitObjectLifetimeDefault
| LifetimeName::Implicit
| LifetimeName::Error => Ident::invalid(),
LifetimeName::Underscore => Ident::with_dummy_span(kw::UnderscoreLifetime),
LifetimeName::Static => Ident::with_dummy_span(kw::StaticLifetime),
LifetimeName::Param(param_name) => param_name.ident(),
Expand All @@ -244,7 +259,9 @@ impl LifetimeName {

pub fn is_elided(&self) -> bool {
match self {
LifetimeName::Implicit | LifetimeName::Underscore => true,
LifetimeName::ImplicitObjectLifetimeDefault
| LifetimeName::Implicit
| LifetimeName::Underscore => true,

// It might seem surprising that `Fresh(_)` counts as
// *not* elided -- but this is because, as far as the code
Expand Down
1 change: 1 addition & 0 deletions src/librustc/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1108,6 +1108,7 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
// Use the same type variable if the exact same opaque type appears more
// than once in the return type (e.g., if it's passed to a type alias).
if let Some(opaque_defn) = self.opaque_types.get(&def_id) {
debug!("instantiate_opaque_types: returning concrete ty {:?}", opaque_defn.concrete_ty);
return opaque_defn.concrete_ty;
}
let span = tcx.def_span(def_id);
Expand Down
93 changes: 92 additions & 1 deletion src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
//! used between functions, and they operate in a purely top-down
//! way. Therefore, we break lifetime name resolution into a separate pass.

// ignore-tidy-filelength

use crate::hir::def::{Res, DefKind};
use crate::hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE};
use crate::hir::map::Map;
Expand Down Expand Up @@ -556,6 +558,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {

fn visit_ty(&mut self, ty: &'tcx hir::Ty) {
debug!("visit_ty: id={:?} ty={:?}", ty.hir_id, ty);
debug!("visit_ty: ty.node={:?}", ty.node);
match ty.node {
hir::TyKind::BareFn(ref c) => {
let next_early_index = self.next_early_index();
Expand Down Expand Up @@ -585,11 +588,20 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
self.is_in_fn_syntax = was_in_fn_syntax;
}
hir::TyKind::TraitObject(ref bounds, ref lifetime) => {
debug!("visit_ty: TraitObject(bounds={:?}, lifetime={:?})", bounds, lifetime);
for bound in bounds {
self.visit_poly_trait_ref(bound, hir::TraitBoundModifier::None);
}
match lifetime.name {
LifetimeName::Implicit => {
// For types like `dyn Foo`, we should
// generate a special form of elided.
span_bug!(
ty.span,
"object-lifetime-default expected, not implict",
);
}
LifetimeName::ImplicitObjectLifetimeDefault => {
// If the user does not write *anything*, we
// use the object lifetime defaulting
// rules. So e.g., `Box<dyn Debug>` becomes
Expand Down Expand Up @@ -897,6 +909,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
}

fn visit_lifetime(&mut self, lifetime_ref: &'tcx hir::Lifetime) {
debug!("visit_lifetime(lifetime_ref={:?})", lifetime_ref);
if lifetime_ref.is_elided() {
self.resolve_elided_lifetimes(vec![lifetime_ref]);
return;
Expand Down Expand Up @@ -1911,6 +1924,13 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}

fn visit_segment_args(&mut self, res: Res, depth: usize, generic_args: &'tcx hir::GenericArgs) {
debug!(
"visit_segment_args(res={:?}, depth={:?}, generic_args={:?})",
res,
depth,
generic_args,
);

if generic_args.parenthesized {
let was_in_fn_syntax = self.is_in_fn_syntax;
self.is_in_fn_syntax = true;
Expand Down Expand Up @@ -1964,6 +1984,23 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
_ => None,
};

debug!("visit_segment_args: type_def_id={:?}", type_def_id);

// Compute a vector of defaults, one for each type parameter,
// per the rules given in RFCs 599 and 1156. Example:
//
// ```rust
// struct Foo<'a, T: 'a, U> { }
// ```
//
// If you have `Foo<'x, dyn Bar, dyn Baz>`, we want to default
// `dyn Bar` to `dyn Bar + 'x` (because of the `T: 'a` bound)
// and `dyn Baz` to `dyn Baz + 'static` (because there is no
// such bound).
//
// Therefore, we would compute `object_lifetime_defaults` to a
// vector like `['x, 'static]`. Note that the vector only
// includes type parameters.
let object_lifetime_defaults = type_def_id.map_or(vec![], |def_id| {
let in_body = {
let mut scope = self.scope;
Expand Down Expand Up @@ -2003,6 +2040,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
.collect()
})
};
debug!("visit_segment_args: unsubst={:?}", unsubst);
unsubst
.iter()
.map(|set| match *set {
Expand All @@ -2023,6 +2061,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
.collect()
});

debug!("visit_segment_args: object_lifetime_defaults={:?}", object_lifetime_defaults);

let mut i = 0;
for arg in &generic_args.args {
match arg {
Expand All @@ -2045,8 +2085,49 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
}

// Hack: when resolving the type `XX` in binding like `dyn
// Foo<'b, Item = XX>`, the current object-lifetime default
// would be to examine the trait `Foo` to check whether it has
// a lifetime bound declared on `Item`. e.g., if `Foo` is
// declared like so, then the default object lifetime bound in
// `XX` should be `'b`:
//
// ```rust
// trait Foo<'a> {
// type Item: 'a;
// }
// ```
//
// but if we just have `type Item;`, then it would be
// `'static`. However, we don't get all of this logic correct.
//
// Instead, we do something hacky: if there are no lifetime parameters
// to the trait, then we simply use a default object lifetime
// bound of `'static`, because there is no other possibility. On the other hand,
// if there ARE lifetime parameters, then we require the user to give an
// explicit bound for now.
//
// This is intended to leave room for us to implement the
// correct behavior in the future.
let has_lifetime_parameter = generic_args
.args
.iter()
.any(|arg| match arg {
GenericArg::Lifetime(_) => true,
_ => false,
});

// Resolve lifetimes found in the type `XX` from `Item = XX` bindings.
for b in &generic_args.bindings {
self.visit_assoc_type_binding(b);
let scope = Scope::ObjectLifetimeDefault {
lifetime: if has_lifetime_parameter {
None
} else {
Some(Region::Static)
},
s: self.scope,
};
self.with(scope, |_, this| this.visit_assoc_type_binding(b));
}
}

Expand Down Expand Up @@ -2347,6 +2428,8 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}

fn resolve_elided_lifetimes(&mut self, lifetime_refs: Vec<&'tcx hir::Lifetime>) {
debug!("resolve_elided_lifetimes(lifetime_refs={:?})", lifetime_refs);

if lifetime_refs.is_empty() {
return;
}
Expand Down Expand Up @@ -2539,6 +2622,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}

fn resolve_object_lifetime_default(&mut self, lifetime_ref: &'tcx hir::Lifetime) {
debug!("resolve_object_lifetime_default(lifetime_ref={:?})", lifetime_ref);
let mut late_depth = 0;
let mut scope = self.scope;
let lifetime = loop {
Expand Down Expand Up @@ -2638,6 +2722,13 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
hir::LifetimeName::Param(_) | hir::LifetimeName::Implicit => {
self.resolve_lifetime_ref(lt);
}
hir::LifetimeName::ImplicitObjectLifetimeDefault => {
self.tcx.sess.delay_span_bug(
lt.span,
"lowering generated `ImplicitObjectLifetimeDefault` \
outside of an object type",
)
}
hir::LifetimeName::Error => {
// No need to do anything, error already reported.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
})
}

hir::LifetimeName::Implicit => {
hir::LifetimeName::ImplicitObjectLifetimeDefault
| hir::LifetimeName::Implicit => {
// In this case, the user left off the lifetime; so
// they wrote something like:
//
Expand Down
Loading

0 comments on commit bece117

Please sign in to comment.