Skip to content

Commit

Permalink
Forbid lifetime elision in let position impl Trait
Browse files Browse the repository at this point in the history
This is consistent with types.
  • Loading branch information
matthewjasper committed Jun 11, 2020
1 parent 4201fd2 commit f97070d
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 52 deletions.
6 changes: 1 addition & 5 deletions src/librustc_ast_lowering/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1619,15 +1619,11 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
visitor.visit_ty(ty);
}
}
let parent_def_id = self.current_hir_id_owner.last().unwrap().0;
let ty = l.ty.as_ref().map(|t| {
self.lower_ty(
t,
if self.sess.features_untracked().impl_trait_in_bindings {
ImplTraitContext::OpaqueTy(
Some(parent_def_id.to_def_id()),
hir::OpaqueTyOrigin::Misc,
)
ImplTraitContext::OpaqueTy(None, hir::OpaqueTyOrigin::Binding)
} else {
ImplTraitContext::Disallowed(ImplTraitPosition::Binding)
},
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_hir/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2018,7 +2018,9 @@ pub enum OpaqueTyOrigin {
FnReturn,
/// `async fn`
AsyncFn,
/// Impl trait in bindings, consts, statics, bounds.
/// `let _: impl Trait = ...`
Binding,
/// Impl trait in type aliases, consts, statics, bounds.
Misc,
}

Expand Down
13 changes: 12 additions & 1 deletion src/librustc_resolve/late/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ enum Elide {
Exact(Region),
/// Less or more than one lifetime were found, error on unspecified.
Error(Vec<ElisionFailureInfo>),
/// Forbid lifetime elision inside of a larger scope that does. For
/// example, in let position impl trait.
Forbid,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -566,7 +569,14 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
// This arm is for `impl Trait` in the types of statics, constants and locals.
hir::ItemKind::OpaqueTy(hir::OpaqueTy { impl_trait_fn: None, .. }) => {
intravisit::walk_ty(self, ty);
intravisit::walk_item(this, opaque_ty);

// Elided lifetimes are not allowed in non-return
// position impl Trait
let scope = Scope::Elision { elide: Elide::Forbid, s: self.scope };
self.with(scope, |_, this| {
intravisit::walk_item(this, opaque_ty);
});

return;
}
// RPIT (return position impl trait)
Expand Down Expand Up @@ -2332,6 +2342,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
break Some(e);
}
Elide::Forbid => break None,
};
for lifetime_ref in lifetime_refs {
self.insert_lifetime(lifetime_ref, lifetime);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_trait_selection/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}
// These opaque type inherit all lifetime parameters from their
// parent.
hir::OpaqueTyOrigin::Misc => 0,
hir::OpaqueTyOrigin::Binding | hir::OpaqueTyOrigin::Misc => 0,
};

let span = tcx.def_span(def_id);
Expand Down Expand Up @@ -569,7 +569,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
hir::OpaqueTyOrigin::AsyncFn => return false,

// Otherwise, generate the label we'll use in the error message.
hir::OpaqueTyOrigin::TypeAlias
hir::OpaqueTyOrigin::Binding
| hir::OpaqueTyOrigin::FnReturn
| hir::OpaqueTyOrigin::Misc => "impl Trait",
};
Expand Down
74 changes: 51 additions & 23 deletions src/librustc_typeck/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,26 +100,17 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
let substs = InternalSubsts::identity_for_item(tcx, def_id);
tcx.mk_adt(def, substs)
}
ItemKind::OpaqueTy(OpaqueTy { origin: hir::OpaqueTyOrigin::Binding, .. }) => {
let_position_impl_trait_type(tcx, def_id.expect_local())
}
ItemKind::OpaqueTy(OpaqueTy { impl_trait_fn: None, .. }) => {
find_opaque_ty_constraints(tcx, def_id.expect_local())
}
// Opaque types desugared from `impl Trait`.
ItemKind::OpaqueTy(OpaqueTy { impl_trait_fn: Some(owner), origin, .. }) => {
let concrete_types = match origin {
OpaqueTyOrigin::FnReturn | OpaqueTyOrigin::AsyncFn => {
&tcx.mir_borrowck(owner.expect_local()).concrete_opaque_types
}
OpaqueTyOrigin::Misc => {
// We shouldn't leak borrowck results through impl trait in bindings.
// For example, we shouldn't be able to tell if `x` in
// `let x: impl Sized + 'a = &()` has type `&'static ()` or `&'a ()`.
&tcx.typeck_tables_of(owner.expect_local()).concrete_opaque_types
}
OpaqueTyOrigin::TypeAlias => {
span_bug!(item.span, "Type alias impl trait shouldn't have an owner")
}
};
let concrete_ty = concrete_types
ItemKind::OpaqueTy(OpaqueTy { impl_trait_fn: Some(owner), .. }) => {
let concrete_ty = tcx
.mir_borrowck(owner.expect_local())
.concrete_opaque_types
.get(&def_id)
.map(|opaque| opaque.concrete_type)
.unwrap_or_else(|| {
Expand Down Expand Up @@ -148,13 +139,6 @@ pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> {
}
});
debug!("concrete_ty = {:?}", concrete_ty);
if concrete_ty.has_erased_regions() {
// FIXME(impl_trait_in_bindings) Handle this case.
tcx.sess.span_fatal(
item.span,
"lifetimes in impl Trait types in bindings are not currently supported",
);
}
concrete_ty
}
ItemKind::Trait(..)
Expand Down Expand Up @@ -589,6 +573,50 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Ty<'_> {
}
}

fn let_position_impl_trait_type(tcx: TyCtxt<'_>, opaque_ty_id: LocalDefId) -> Ty<'_> {
let scope = tcx.hir().get_defining_scope(tcx.hir().as_local_hir_id(opaque_ty_id));
let scope_def_id = tcx.hir().local_def_id(scope);

let opaque_ty_def_id = opaque_ty_id.to_def_id();

let owner_tables = tcx.typeck_tables_of(scope_def_id);
let concrete_ty = owner_tables
.concrete_opaque_types
.get(&opaque_ty_def_id)
.map(|opaque| opaque.concrete_type)
.unwrap_or_else(|| {
tcx.sess.delay_span_bug(
DUMMY_SP,
&format!(
"owner {:?} has no opaque type for {:?} in its tables",
scope_def_id, opaque_ty_id
),
);
if let Some(ErrorReported) = owner_tables.tainted_by_errors {
// Some error in the owner fn prevented us from populating the
// `concrete_opaque_types` table.
tcx.types.err
} else {
// We failed to resolve the opaque type or it resolves to
// itself. Return the non-revealed type, which should result in
// E0720.
tcx.mk_opaque(
opaque_ty_def_id,
InternalSubsts::identity_for_item(tcx, opaque_ty_def_id),
)
}
});
debug!("concrete_ty = {:?}", concrete_ty);
if concrete_ty.has_erased_regions() {
// FIXME(impl_trait_in_bindings) Handle this case.
tcx.sess.span_fatal(
tcx.hir().span(tcx.hir().as_local_hir_id(opaque_ty_id)),
"lifetimes in impl Trait types in bindings are not currently supported",
);
}
concrete_ty
}

fn infer_placeholder_type(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
Expand Down
10 changes: 4 additions & 6 deletions src/test/ui/impl-trait/issue-60473.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,11 @@

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

trait Trait<T> {
}
trait Trait<T> {}

impl<T> Trait<T> for () {
}
impl<T> Trait<T> for () {}

fn main() {
let x: impl Trait<A> = (); // FIXME: The error doesn't seem correct.
//~^ ERROR: opaque type expands to a recursive type
let x: impl Trait<A> = ();
//~^ ERROR: missing lifetime specifier
}
16 changes: 10 additions & 6 deletions src/test/ui/impl-trait/issue-60473.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
error[E0720]: opaque type expands to a recursive type
--> $DIR/issue-60473.rs:15:12
error[E0106]: missing lifetime specifier
--> $DIR/issue-60473.rs:13:23
|
LL | let x: impl Trait<A> = (); // FIXME: The error doesn't seem correct.
| ^^^^^^^^^^^^^ expands to a recursive type
LL | let x: impl Trait<A> = ();
| ^ expected named lifetime parameter
|
help: consider introducing a named lifetime parameter
|
LL | fn main<'a>() {
LL | let x: impl Trait<A<'a>> = ();
|
= note: type resolves to itself

error: aborting due to previous error

For more information about this error, try `rustc --explain E0720`.
For more information about this error, try `rustc --explain E0106`.
4 changes: 2 additions & 2 deletions src/test/ui/impl-trait/issue-67166.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
#![allow(incomplete_features)]

pub fn run() {
let _foo: Box<impl Copy + '_> = Box::new(()); // FIXME: The error doesn't much make sense.
//~^ ERROR: opaque type expands to a recursive type
let _foo: Box<impl Copy + '_> = Box::new(());
//~^ ERROR: missing lifetime specifier
}

fn main() {}
16 changes: 10 additions & 6 deletions src/test/ui/impl-trait/issue-67166.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
error[E0720]: opaque type expands to a recursive type
--> $DIR/issue-67166.rs:7:19
error[E0106]: missing lifetime specifier
--> $DIR/issue-67166.rs:7:31
|
LL | let _foo: Box<impl Copy + '_> = Box::new(()); // FIXME: The error doesn't much make sense.
| ^^^^^^^^^^^^^^ expands to a recursive type
LL | let _foo: Box<impl Copy + '_> = Box::new(());
| ^^ expected named lifetime parameter
|
help: consider introducing a named lifetime parameter
|
LL | pub fn run<'a>() {
LL | let _foo: Box<impl Copy + 'a> = Box::new(());
|
= note: type resolves to itself

error: aborting due to previous error

For more information about this error, try `rustc --explain E0720`.
For more information about this error, try `rustc --explain E0106`.

0 comments on commit f97070d

Please sign in to comment.