Skip to content

Commit

Permalink
Auto merge of #55162 - nikomatsakis:issue-54902-underscore-bound, r=t…
Browse files Browse the repository at this point in the history
…mandry

handle underscore bounds in unexpected places

Per the discussion on #54902, I made it a hard error to use lifetime bounds in various places where they used to be permitted:

- `where Foo: Bar<'_>` for example

I also moved error reporting to HIR lowering and added `Error` variants to let us suppress downstream errors that result.

I (imo) improved the error message wording to be clearer, as well.

In the process, I fixed the ICE in #52098.

Fixes #54902
Fixes #52098
  • Loading branch information
bors committed Oct 19, 2018
2 parents 78ff609 + c294ec6 commit 42dde96
Show file tree
Hide file tree
Showing 36 changed files with 910 additions and 362 deletions.
4 changes: 3 additions & 1 deletion src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,9 @@ pub fn walk_lifetime<'v, V: Visitor<'v>>(visitor: &mut V, lifetime: &'v Lifetime
visitor.visit_ident(ident);
}
LifetimeName::Param(ParamName::Fresh(_)) |
LifetimeName::Param(ParamName::Error) |
LifetimeName::Static |
LifetimeName::Error |
LifetimeName::Implicit |
LifetimeName::Underscore => {}
}
Expand Down Expand Up @@ -747,7 +749,7 @@ pub fn walk_generic_param<'v, V: Visitor<'v>>(visitor: &mut V, param: &'v Generi
walk_list!(visitor, visit_attribute, &param.attrs);
match param.name {
ParamName::Plain(ident) => visitor.visit_ident(ident),
ParamName::Fresh(_) => {}
ParamName::Error | ParamName::Fresh(_) => {}
}
match param.kind {
GenericParamKind::Lifetime { .. } => {}
Expand Down
100 changes: 83 additions & 17 deletions src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,11 @@ enum AnonymousLifetimeMode {
/// For **Deprecated** cases, report an error.
CreateParameter,

/// Give a hard error when either `&` or `'_` is written. Used to
/// rule out things like `where T: Foo<'_>`. Does not imply an
/// error on default object bounds (e.g., `Box<dyn Foo>`).
ReportError,

/// Pass responsibility to `resolve_lifetime` code for all cases.
PassThrough,
}
Expand Down Expand Up @@ -735,6 +740,10 @@ impl<'a> LoweringContext<'a> {
keywords::UnderscoreLifetime.name().as_interned_str(),
hir::LifetimeParamKind::Elided,
),
ParamName::Error => (
keywords::UnderscoreLifetime.name().as_interned_str(),
hir::LifetimeParamKind::Error,
),
};

// Add a definition for the in-band lifetime def
Expand Down Expand Up @@ -791,7 +800,7 @@ impl<'a> LoweringContext<'a> {
}

/// When we have either an elided or `'_` lifetime in an impl
/// header, we convert it to
/// header, we convert it to an in-band lifetime.
fn collect_fresh_in_band_lifetime(&mut self, span: Span) -> ParamName {
assert!(self.is_collecting_in_band_lifetimes);
let index = self.lifetimes_to_define.len();
Expand Down Expand Up @@ -1474,7 +1483,7 @@ impl<'a> LoweringContext<'a> {
}
}
hir::LifetimeName::Param(_) => lifetime.name,
hir::LifetimeName::Static => return,
hir::LifetimeName::Error | hir::LifetimeName::Static => return,
};

if !self.currently_bound_lifetimes.contains(&name)
Expand Down Expand Up @@ -2162,7 +2171,7 @@ impl<'a> LoweringContext<'a> {
}
}
hir::LifetimeName::Param(_) => lifetime.name,
hir::LifetimeName::Static => return,
hir::LifetimeName::Error | hir::LifetimeName::Static => return,
};

if !self.currently_bound_lifetimes.contains(&name) {
Expand Down Expand Up @@ -2293,10 +2302,12 @@ impl<'a> LoweringContext<'a> {
itctx: ImplTraitContext<'_>,
) -> hir::GenericBound {
match *tpb {
GenericBound::Trait(ref ty, modifier) => hir::GenericBound::Trait(
self.lower_poly_trait_ref(ty, itctx),
self.lower_trait_bound_modifier(modifier),
),
GenericBound::Trait(ref ty, modifier) => {
hir::GenericBound::Trait(
self.lower_poly_trait_ref(ty, itctx),
self.lower_trait_bound_modifier(modifier),
)
}
GenericBound::Outlives(ref lifetime) => {
hir::GenericBound::Outlives(self.lower_lifetime(lifetime))
}
Expand All @@ -2318,6 +2329,8 @@ impl<'a> LoweringContext<'a> {
AnonymousLifetimeMode::PassThrough => {
self.new_named_lifetime(l.id, span, hir::LifetimeName::Underscore)
}

AnonymousLifetimeMode::ReportError => self.new_error_lifetime(Some(l.id), span),
},
ident => {
self.maybe_collect_in_band_lifetime(ident);
Expand Down Expand Up @@ -2356,16 +2369,26 @@ impl<'a> LoweringContext<'a> {
add_bounds: &NodeMap<Vec<GenericBound>>,
mut itctx: ImplTraitContext<'_>)
-> hir::GenericParam {
let mut bounds = self.lower_param_bounds(&param.bounds, itctx.reborrow());
let mut bounds = self.with_anonymous_lifetime_mode(
AnonymousLifetimeMode::ReportError,
|this| this.lower_param_bounds(&param.bounds, itctx.reborrow()),
);

match param.kind {
GenericParamKind::Lifetime => {
let was_collecting_in_band = self.is_collecting_in_band_lifetimes;
self.is_collecting_in_band_lifetimes = false;

let lt = self.lower_lifetime(&Lifetime { id: param.id, ident: param.ident });
let lt = self.with_anonymous_lifetime_mode(
AnonymousLifetimeMode::ReportError,
|this| this.lower_lifetime(&Lifetime { id: param.id, ident: param.ident }),
);
let param_name = match lt.name {
hir::LifetimeName::Param(param_name) => param_name,
_ => hir::ParamName::Plain(lt.name.ident()),
hir::LifetimeName::Implicit
| hir::LifetimeName::Underscore
| hir::LifetimeName::Static => hir::ParamName::Plain(lt.name.ident()),
hir::LifetimeName::Error => ParamName::Error,
};
let param = hir::GenericParam {
id: lt.id,
Expand Down Expand Up @@ -2489,13 +2512,18 @@ impl<'a> LoweringContext<'a> {
}

fn lower_where_clause(&mut self, wc: &WhereClause) -> hir::WhereClause {
hir::WhereClause {
id: self.lower_node_id(wc.id).node_id,
predicates: wc.predicates
.iter()
.map(|predicate| self.lower_where_predicate(predicate))
.collect(),
}
self.with_anonymous_lifetime_mode(
AnonymousLifetimeMode::ReportError,
|this| {
hir::WhereClause {
id: this.lower_node_id(wc.id).node_id,
predicates: wc.predicates
.iter()
.map(|predicate| this.lower_where_predicate(predicate))
.collect(),
}
},
)
}

fn lower_where_predicate(&mut self, pred: &WherePredicate) -> hir::WherePredicate {
Expand Down Expand Up @@ -4837,10 +4865,38 @@ impl<'a> LoweringContext<'a> {
}
}

AnonymousLifetimeMode::ReportError => self.new_error_lifetime(None, span),

AnonymousLifetimeMode::PassThrough => self.new_implicit_lifetime(span),
}
}

/// Report an error on illegal use of `'_` or a `&T` with no explicit lifetime;
/// return a "error lifetime".
fn new_error_lifetime(&mut self, id: Option<NodeId>, span: Span) -> hir::Lifetime {
let (id, msg, label) = match id {
Some(id) => (id, "`'_` cannot be used here", "`'_` is a reserved lifetime name"),

None => (
self.next_id().node_id,
"`&` without an explicit lifetime name cannot be used here",
"explicit lifetime name needed here",
),
};

let mut err = struct_span_err!(
self.sess,
span,
E0637,
"{}",
msg,
);
err.span_label(span, label);
err.emit();

self.new_named_lifetime(id, span, hir::LifetimeName::Error)
}

/// Invoked to create the lifetime argument(s) for a path like
/// `std::cell::Ref<T>`; note that implicit lifetimes in these
/// sorts of cases are deprecated. This may therefore report a warning or an
Expand All @@ -4855,6 +4911,12 @@ impl<'a> LoweringContext<'a> {
// impl Foo for std::cell::Ref<u32> // note lack of '_
AnonymousLifetimeMode::CreateParameter => {}

AnonymousLifetimeMode::ReportError => {
return (0..count)
.map(|_| self.new_error_lifetime(None, span))
.collect();
}

// This is the normal case.
AnonymousLifetimeMode::PassThrough => {}
}
Expand Down Expand Up @@ -4885,6 +4947,10 @@ impl<'a> LoweringContext<'a> {
// `resolve_lifetime` has the code to make that happen.
AnonymousLifetimeMode::CreateParameter => {}

AnonymousLifetimeMode::ReportError => {
// ReportError applies to explicit use of `'_`.
}

// This is the normal case.
AnonymousLifetimeMode::PassThrough => {}
}
Expand Down
17 changes: 15 additions & 2 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,13 +208,18 @@ pub enum ParamName {
/// where `'f` is something like `Fresh(0)`. The indices are
/// unique per impl, but not necessarily continuous.
Fresh(usize),

/// Indicates an illegal name was given and an error has been
/// repored (so we should squelch other derived errors). Occurs
/// when e.g. `'_` is used in the wrong place.
Error,
}

impl ParamName {
pub fn ident(&self) -> Ident {
match *self {
ParamName::Plain(ident) => ident,
ParamName::Fresh(_) => keywords::UnderscoreLifetime.ident(),
ParamName::Error | ParamName::Fresh(_) => keywords::UnderscoreLifetime.ident(),
}
}

Expand All @@ -234,6 +239,10 @@ pub enum LifetimeName {
/// User typed nothing. e.g. the lifetime in `&u32`.
Implicit,

/// Indicates an error during lowering (usually `'_` in wrong place)
/// that was already reported.
Error,

/// User typed `'_`.
Underscore,

Expand All @@ -245,6 +254,7 @@ impl LifetimeName {
pub fn ident(&self) -> Ident {
match *self {
LifetimeName::Implicit => keywords::Invalid.ident(),
LifetimeName::Error => keywords::Invalid.ident(),
LifetimeName::Underscore => keywords::UnderscoreLifetime.ident(),
LifetimeName::Static => keywords::StaticLifetime.ident(),
LifetimeName::Param(param_name) => param_name.ident(),
Expand All @@ -260,7 +270,7 @@ impl LifetimeName {
// in the compiler is concerned -- `Fresh(_)` variants act
// equivalently to "some fresh name". They correspond to
// early-bound regions on an impl, in other words.
LifetimeName::Param(_) | LifetimeName::Static => false,
LifetimeName::Error | LifetimeName::Param(_) | LifetimeName::Static => false,
}
}

Expand Down Expand Up @@ -513,6 +523,9 @@ pub enum LifetimeParamKind {
// Indication that the lifetime was elided like both cases here:
// `fn foo(x: &u8) -> &'_ u8 { x }`
Elided,

// Indication that the lifetime name was somehow in error.
Error,
}

#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
Expand Down
7 changes: 5 additions & 2 deletions src/librustc/ich/impls_hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,16 @@ impl<'a> HashStable<StableHashingContext<'a>> for hir::ImplItemId {

impl_stable_hash_for!(enum hir::ParamName {
Plain(name),
Fresh(index)
Fresh(index),
Error,
});

impl_stable_hash_for!(enum hir::LifetimeName {
Param(param_name),
Implicit,
Underscore,
Static,
Error,
});

impl_stable_hash_for!(struct hir::Label {
Expand Down Expand Up @@ -210,7 +212,8 @@ impl_stable_hash_for!(struct hir::GenericParam {
impl_stable_hash_for!(enum hir::LifetimeParamKind {
Explicit,
InBand,
Elided
Elided,
Error,
});

impl<'a> HashStable<StableHashingContext<'a>> for hir::GenericParamKind {
Expand Down
5 changes: 3 additions & 2 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -755,8 +755,9 @@ for ::middle::resolve_lifetime::Set1<T>
}

impl_stable_hash_for!(enum ::middle::resolve_lifetime::LifetimeDefOrigin {
Explicit,
InBand
ExplicitOrElided,
InBand,
Error,
});

impl_stable_hash_for!(enum ::middle::resolve_lifetime::Region {
Expand Down
Loading

0 comments on commit 42dde96

Please sign in to comment.