Skip to content

Commit

Permalink
Auto merge of #114489 - compiler-errors:rpitit-capture-all, r=oli-obk
Browse files Browse the repository at this point in the history
Make RPITITs capture all in-scope lifetimes

Much like #114616, this implements the lang team decision from this T-lang meeting on [opaque captures strategy moving forward](https://hackmd.io/sFaSIMJOQcuwCdnUvCxtuQ?view). This will be RFC'd soon, but given that RPITITs are a nightly feature, this shouldn't necessarily be blocked on that.

We unconditionally capture all lifetimes in RPITITs -- impl is not as simple as #114616, since we still need to duplicate RPIT lifetimes to make sure we reify any late-bound lifetimes in scope.

Closes #112194
  • Loading branch information
bors committed Aug 28, 2023
2 parents 9f48a85 + f8e0dcb commit 93dd620
Show file tree
Hide file tree
Showing 15 changed files with 207 additions and 105 deletions.
53 changes: 31 additions & 22 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ enum ImplTraitContext {
ReturnPositionOpaqueTy {
/// Origin: Either OpaqueTyOrigin::FnReturn or OpaqueTyOrigin::AsyncFn,
origin: hir::OpaqueTyOrigin,
in_trait: bool,
fn_kind: FnDeclKind,
},
/// Impl trait in type aliases.
TypeAliasesOpaqueTy { in_assoc_ty: bool },
Expand Down Expand Up @@ -312,7 +312,7 @@ impl std::fmt::Display for ImplTraitPosition {
}
}

#[derive(Debug, PartialEq, Eq)]
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
enum FnDeclKind {
Fn,
Inherent,
Expand Down Expand Up @@ -1401,13 +1401,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
TyKind::ImplTrait(def_node_id, bounds) => {
let span = t.span;
match itctx {
ImplTraitContext::ReturnPositionOpaqueTy { origin, in_trait } => self
ImplTraitContext::ReturnPositionOpaqueTy { origin, fn_kind } => self
.lower_opaque_impl_trait(
span,
*origin,
*def_node_id,
bounds,
*in_trait,
Some(*fn_kind),
itctx,
),
&ImplTraitContext::TypeAliasesOpaqueTy { in_assoc_ty } => self
Expand All @@ -1416,7 +1416,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
hir::OpaqueTyOrigin::TyAlias { in_assoc_ty },
*def_node_id,
bounds,
false,
None,
itctx,
),
ImplTraitContext::Universal => {
Expand Down Expand Up @@ -1523,7 +1523,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
origin: hir::OpaqueTyOrigin,
opaque_ty_node_id: NodeId,
bounds: &GenericBounds,
in_trait: bool,
fn_kind: Option<FnDeclKind>,
itctx: &ImplTraitContext,
) -> hir::TyKind<'hir> {
// Make sure we know that some funky desugaring has been going on here.
Expand All @@ -1540,10 +1540,22 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
Vec::new()
}
hir::OpaqueTyOrigin::FnReturn(..) => {
// in fn return position, like the `fn test<'a>() -> impl Debug + 'a`
// example, we only need to duplicate lifetimes that appear in the
// bounds, since those are the only ones that are captured by the opaque.
lifetime_collector::lifetimes_in_bounds(&self.resolver, bounds)
if let FnDeclKind::Impl | FnDeclKind::Trait =
fn_kind.expect("expected RPITs to be lowered with a FnKind")
{
// return-position impl trait in trait was decided to capture all
// in-scope lifetimes, which we collect for all opaques during resolution.
self.resolver
.take_extra_lifetime_params(opaque_ty_node_id)
.into_iter()
.map(|(ident, id, _)| Lifetime { id, ident })
.collect()
} else {
// in fn return position, like the `fn test<'a>() -> impl Debug + 'a`
// example, we only need to duplicate lifetimes that appear in the
// bounds, since those are the only ones that are captured by the opaque.
lifetime_collector::lifetimes_in_bounds(&self.resolver, bounds)
}
}
hir::OpaqueTyOrigin::AsyncFn(..) => {
unreachable!("should be using `lower_async_fn_ret_ty`")
Expand All @@ -1554,7 +1566,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
self.lower_opaque_inner(
opaque_ty_node_id,
origin,
in_trait,
matches!(fn_kind, Some(FnDeclKind::Trait)),
captured_lifetimes_to_duplicate,
span,
opaque_ty_span,
Expand Down Expand Up @@ -1802,20 +1814,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}

let fn_def_id = self.local_def_id(fn_node_id);
self.lower_async_fn_ret_ty(
&decl.output,
fn_def_id,
ret_id,
matches!(kind, FnDeclKind::Trait),
)
self.lower_async_fn_ret_ty(&decl.output, fn_def_id, ret_id, kind)
} else {
match &decl.output {
FnRetTy::Ty(ty) => {
let context = if kind.return_impl_trait_allowed(self.tcx) {
let fn_def_id = self.local_def_id(fn_node_id);
ImplTraitContext::ReturnPositionOpaqueTy {
origin: hir::OpaqueTyOrigin::FnReturn(fn_def_id),
in_trait: matches!(kind, FnDeclKind::Trait),
fn_kind: kind,
}
} else {
let position = match kind {
Expand Down Expand Up @@ -1883,7 +1890,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
output: &FnRetTy,
fn_def_id: LocalDefId,
opaque_ty_node_id: NodeId,
in_trait: bool,
fn_kind: FnDeclKind,
) -> hir::FnRetTy<'hir> {
let span = self.lower_span(output.span());
let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::Async, span, None);
Expand All @@ -1898,23 +1905,25 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let opaque_ty_ref = self.lower_opaque_inner(
opaque_ty_node_id,
hir::OpaqueTyOrigin::AsyncFn(fn_def_id),
in_trait,
matches!(fn_kind, FnDeclKind::Trait),
captured_lifetimes,
span,
opaque_ty_span,
|this| {
let future_bound = this.lower_async_fn_output_type_to_future_bound(
output,
span,
if in_trait && !this.tcx.features().return_position_impl_trait_in_trait {
if let FnDeclKind::Trait = fn_kind
&& !this.tcx.features().return_position_impl_trait_in_trait
{
ImplTraitContext::FeatureGated(
ImplTraitPosition::TraitReturn,
sym::return_position_impl_trait_in_trait,
)
} else {
ImplTraitContext::ReturnPositionOpaqueTy {
origin: hir::OpaqueTyOrigin::FnReturn(fn_def_id),
in_trait,
fn_kind,
}
},
);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_strict_coherence, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_variance, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_variance_of_opaques, Normal, template!(Word), WarnFollowing),
rustc_attr!(TEST, rustc_layout, Normal, template!(List: "field1, field2, ..."), WarnFollowing),
rustc_attr!(TEST, rustc_abi, Normal, template!(List: "field1, field2, ..."), WarnFollowing),
rustc_attr!(TEST, rustc_regions, Normal, template!(Word), WarnFollowing),
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_hir_analysis/src/variance/test.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,24 @@
use rustc_hir::def::DefKind;
use rustc_hir::def_id::CRATE_DEF_ID;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::sym;

use crate::errors;

pub fn test_variance(tcx: TyCtxt<'_>) {
if tcx.has_attr(CRATE_DEF_ID, sym::rustc_variance_of_opaques) {
for id in tcx.hir().items() {
if matches!(tcx.def_kind(id.owner_id), DefKind::OpaqueTy) {
let variances_of = tcx.variances_of(id.owner_id);

tcx.sess.emit_err(errors::VariancesOf {
span: tcx.def_span(id.owner_id),
variances_of: format!("{variances_of:?}"),
});
}
}
}

// For unit testing: check for a special "rustc_variance"
// attribute and report an error with various results if found.
for id in tcx.hir().items() {
Expand Down
21 changes: 9 additions & 12 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -772,9 +772,10 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast,
self.r.record_partial_res(ty.id, PartialRes::new(res));
visit::walk_ty(self, ty)
}
TyKind::ImplTrait(..) => {
TyKind::ImplTrait(node_id, _) => {
let candidates = self.lifetime_elision_candidates.take();
visit::walk_ty(self, ty);
self.record_lifetime_params_for_impl_trait(*node_id);
self.lifetime_elision_candidates = candidates;
}
TyKind::TraitObject(bounds, ..) => {
Expand Down Expand Up @@ -909,8 +910,8 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast,
&sig.decl.output,
);

if let Some((async_node_id, span)) = sig.header.asyncness.opt_return_id() {
this.record_lifetime_params_for_impl_trait(async_node_id, span);
if let Some((async_node_id, _)) = sig.header.asyncness.opt_return_id() {
this.record_lifetime_params_for_impl_trait(async_node_id);
}
},
);
Expand Down Expand Up @@ -951,8 +952,8 @@ impl<'a: 'ast, 'ast, 'tcx> Visitor<'ast> for LateResolutionVisitor<'a, '_, 'ast,
&declaration.output,
);

if let Some((async_node_id, span)) = async_node_id {
this.record_lifetime_params_for_impl_trait(async_node_id, span);
if let Some((async_node_id, _)) = async_node_id {
this.record_lifetime_params_for_impl_trait(async_node_id);
}
},
);
Expand Down Expand Up @@ -4367,7 +4368,7 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
/// We include all lifetime parameters, either named or "Fresh".
/// The order of those parameters does not matter, as long as it is
/// deterministic.
fn record_lifetime_params_for_impl_trait(&mut self, impl_trait_node_id: NodeId, span: Span) {
fn record_lifetime_params_for_impl_trait(&mut self, impl_trait_node_id: NodeId) {
let mut extra_lifetime_params = vec![];

for rib in self.lifetime_ribs.iter().rev() {
Expand All @@ -4380,14 +4381,10 @@ impl<'a: 'ast, 'b, 'ast, 'tcx> LateResolutionVisitor<'a, 'b, 'ast, 'tcx> {
extra_lifetime_params.extend(earlier_fresh);
}
}
LifetimeRibKind::Generics { .. } => {}
_ => {
// We are in a function definition. We should only find `Generics`
// and `AnonymousCreateParameter` inside the innermost `Item`.
span_bug!(span, "unexpected rib kind: {:?}", rib.kind)
}
_ => {}
}
}

self.r.extra_lifetime_params_map.insert(impl_trait_node_id, extra_lifetime_params);
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@ symbols! {
rustc_trivial_field_reads,
rustc_unsafe_specialization_marker,
rustc_variance,
rustc_variance_of_opaques,
rustdoc,
rustdoc_internals,
rustdoc_missing_doc_code_examples,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation.cause.span,
"GATs in trait object shouldn't have been considered",
);
return Err(SelectionError::Unimplemented);
return Err(SelectionError::TraitNotObjectSafe(trait_predicate.trait_ref.def_id));
}

// This maybe belongs in wf, but that can't (doesn't) handle
Expand Down
1 change: 1 addition & 0 deletions tests/ui/impl-trait/in-trait/object-safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ fn main() {
//~| ERROR the trait `Foo` cannot be made into an object
let s = i.baz();
//~^ ERROR the trait `Foo` cannot be made into an object
//~| ERROR the trait `Foo` cannot be made into an object
}
17 changes: 16 additions & 1 deletion tests/ui/impl-trait/in-trait/object-safety.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,21 @@ LL | fn baz(&self) -> impl Debug;
| ^^^^^^^^^^ ...because method `baz` references an `impl Trait` type in its return type
= help: consider moving `baz` to another trait

error[E0038]: the trait `Foo` cannot be made into an object
--> $DIR/object-safety.rs:20:15
|
LL | let s = i.baz();
| ^^^ `Foo` cannot be made into an object
|
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/object-safety.rs:7:22
|
LL | trait Foo {
| --- this trait cannot be made into an object...
LL | fn baz(&self) -> impl Debug;
| ^^^^^^^^^^ ...because method `baz` references an `impl Trait` type in its return type
= help: consider moving `baz` to another trait

error[E0038]: the trait `Foo` cannot be made into an object
--> $DIR/object-safety.rs:20:13
|
Expand Down Expand Up @@ -44,6 +59,6 @@ LL | fn baz(&self) -> impl Debug;
= help: consider moving `baz` to another trait
= note: required for the cast from `Box<u32>` to `Box<dyn Foo>`

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0038`.
14 changes: 14 additions & 0 deletions tests/ui/impl-trait/in-trait/signature-mismatch.failure.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error[E0623]: lifetime mismatch
--> $DIR/signature-mismatch.rs:77:10
|
LL | &'a self,
| -------- this parameter and the return type are declared with different lifetimes...
...
LL | ) -> impl Future<Output = Vec<u8>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| ...but data from `buff` is returned here

error: aborting due to previous error

For more information about this error, try `rustc --explain E0623`.
Loading

0 comments on commit 93dd620

Please sign in to comment.