Skip to content

Commit

Permalink
Auto merge of #51220 - nikomatsakis:issue-51008-false-positive-lifeti…
Browse files Browse the repository at this point in the history
…me-must-be-declared, r=cramertj

reset anonymous-lifetime-mode as we enter `()` scopes

Background:

The anonymous lifetime mode is used to prohibit elided lifetimes where
they didn't used to be permitted, and instead require that `'_` be
used. For example:

```rust
impl Trait for Ref<T> { .. }
//             ^^^^^^ ERROR: should be `Ref<'_, T>`
```

When we are parsing the parts of the impl header, we enter into an alternate mode called `CreateParameter`. In this mode, we give an error for things like `Ref<T>`, but for elided lifetimes in a reference type like `&T` we make the elided lifetime into an in-band lifetime:

https://github.com/rust-lang/rust/blob/4f99f37b7e213d69a489884f651adfc6d217cef5/src/librustc/hir/lowering.rs#L4017-L4035

This was not intended to change behavior because we only enter into that mode in contexts where elision was not historically permitted. However, the problem is that we fail to reset the mode when we enter into bounds like `Fn(&u32)`, where elision *was* allowed -- the same occurs for fn types like `fn(&u32`). This PR restores the original mode in those contexts.

Fixes #51008

r? @cramertj
  • Loading branch information
bors committed May 31, 2018
2 parents e1eed38 + da69bbc commit c1287c0
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 48 deletions.
123 changes: 75 additions & 48 deletions src/librustc/hir/lowering.rs
Expand Up @@ -594,6 +594,18 @@ impl<'a> LoweringContext<'a> {
span.with_ctxt(SyntaxContext::empty().apply_mark(mark))
}

fn with_anonymous_lifetime_mode<R>(
&mut self,
anonymous_lifetime_mode: AnonymousLifetimeMode,
op: impl FnOnce(&mut Self) -> R,
) -> R {
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;
result
}

/// Creates a new hir::GenericParam for every new lifetime and
/// type parameter encountered while evaluating `f`. Definitions
/// are created with the parent provided. If no `parent_id` is
Expand Down Expand Up @@ -1021,17 +1033,22 @@ impl<'a> LoweringContext<'a> {
_ => None,
}),
|this| {
hir::TyBareFn(P(hir::BareFnTy {
generic_params: this.lower_generic_params(
&f.generic_params,
&NodeMap(),
ImplTraitContext::Disallowed,
),
unsafety: this.lower_unsafety(f.unsafety),
abi: f.abi,
decl: this.lower_fn_decl(&f.decl, None, false),
arg_names: this.lower_fn_args_to_names(&f.decl),
}))
this.with_anonymous_lifetime_mode(
AnonymousLifetimeMode::PassThrough,
|this| {
hir::TyBareFn(P(hir::BareFnTy {
generic_params: this.lower_generic_params(
&f.generic_params,
&NodeMap(),
ImplTraitContext::Disallowed,
),
unsafety: this.lower_unsafety(f.unsafety),
abi: f.abi,
decl: this.lower_fn_decl(&f.decl, None, false),
arg_names: this.lower_fn_args_to_names(&f.decl),
}))
},
)
},
),
TyKind::Never => hir::TyNever,
Expand Down Expand Up @@ -1623,44 +1640,54 @@ impl<'a> LoweringContext<'a> {
&mut self,
data: &ParenthesizedParameterData,
) -> (hir::PathParameters, bool) {
const DISALLOWED: ImplTraitContext = ImplTraitContext::Disallowed;
let &ParenthesizedParameterData {
ref inputs,
ref output,
span,
} = data;
let inputs = inputs
.iter()
.map(|ty| self.lower_ty(ty, DISALLOWED))
.collect();
let mk_tup = |this: &mut Self, tys, span| {
let LoweredNodeId { node_id, hir_id } = this.next_id();
P(hir::Ty {
node: hir::TyTup(tys),
id: node_id,
hir_id,
span,
})
};
// Switch to `PassThrough` mode for anonymous lifetimes: this
// means that we permit things like `&Ref<T>`, where `Ref` has
// a hidden lifetime parameter. This is needed for backwards
// compatibility, even in contexts like an impl header where
// we generally don't permit such things (see #51008).
self.with_anonymous_lifetime_mode(
AnonymousLifetimeMode::PassThrough,
|this| {
const DISALLOWED: ImplTraitContext = ImplTraitContext::Disallowed;
let &ParenthesizedParameterData {
ref inputs,
ref output,
span,
} = data;
let inputs = inputs
.iter()
.map(|ty| this.lower_ty(ty, DISALLOWED))
.collect();
let mk_tup = |this: &mut Self, tys, span| {
let LoweredNodeId { node_id, hir_id } = this.next_id();
P(hir::Ty {
node: hir::TyTup(tys),
id: node_id,
hir_id,
span,
})
};

(
hir::PathParameters {
lifetimes: hir::HirVec::new(),
types: hir_vec![mk_tup(self, inputs, span)],
bindings: hir_vec![
hir::TypeBinding {
id: self.next_id().node_id,
name: Symbol::intern(FN_OUTPUT_NAME),
ty: output
.as_ref()
.map(|ty| self.lower_ty(&ty, DISALLOWED))
.unwrap_or_else(|| mk_tup(self, hir::HirVec::new(), span)),
span: output.as_ref().map_or(span, |ty| ty.span),
}
],
parenthesized: true,
},
false,
(
hir::PathParameters {
lifetimes: hir::HirVec::new(),
types: hir_vec![mk_tup(this, inputs, span)],
bindings: hir_vec![
hir::TypeBinding {
id: this.next_id().node_id,
name: Symbol::intern(FN_OUTPUT_NAME),
ty: output
.as_ref()
.map(|ty| this.lower_ty(&ty, DISALLOWED))
.unwrap_or_else(|| mk_tup(this, hir::HirVec::new(), span)),
span: output.as_ref().map_or(span, |ty| ty.span),
}
],
parenthesized: true,
},
false,
)
}
)
}

Expand Down
7 changes: 7 additions & 0 deletions src/librustc/middle/resolve_lifetime.rs
Expand Up @@ -101,6 +101,13 @@ impl Region {
let depth = ty::DebruijnIndex::INNERMOST;
let def_id = hir_map.local_def_id(def.lifetime.id);
let origin = LifetimeDefOrigin::from_is_in_band(def.in_band);
debug!(
"Region::late: def={:?} depth={:?} def_id={:?} origin={:?}",
def,
depth,
def_id,
origin,
);
(def.lifetime.name, Region::LateBound(depth, def_id, origin))
}

Expand Down
25 changes: 25 additions & 0 deletions src/test/ui/rust-2018/issue-51008-1.rs
@@ -0,0 +1,25 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Regression test for #51008 -- the anonymous lifetime in `&i32` was
// being incorrectly considered part of the "elided lifetimes" from
// the impl.
//
// run-pass

#![feature(rust_2018_preview)]

trait A {

}

impl<F> A for F where F: PartialEq<fn(&i32)> { }

fn main() {}
25 changes: 25 additions & 0 deletions src/test/ui/rust-2018/issue-51008.rs
@@ -0,0 +1,25 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Regression test for #51008 -- the anonymous lifetime in `&i32` was
// being incorrectly considered part of the "elided lifetimes" from
// the impl.
//
// run-pass

#![feature(rust_2018_preview)]

trait A {

}

impl<F> A for F where F: FnOnce(&i32) {}

fn main() {}

0 comments on commit c1287c0

Please sign in to comment.