Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make RPITITs capture all in-scope lifetimes #114489

Merged
merged 4 commits into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading