bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #115659 - compiler-errors:itp, r=cjgillot

Browse source 
Stabilize `impl_trait_projections`

Closes #115659

## TL;DR:

This allows us to mention `Self` and `T::Assoc` in async fn and return-position `impl Trait`, as you would expect you'd be able to.

Some examples:
```rust
#![feature(return_position_impl_trait_in_trait, async_fn_in_trait)]
// (just needed for final tests below)

// ---------------------------------------- //

struct Wrapper<'a, T>(&'a T);

impl Wrapper<'_, ()> {
    async fn async_fn() -> Self {
        //^ Previously rejected because it returns `-> Self`, not `-> Wrapper<'_, ()>`.
        Wrapper(&())
    }

    fn impl_trait() -> impl Iterator<Item = Self> {
        //^ Previously rejected because it mentions `Self`, not `Wrapper<'_, ()>`.
        std::iter::once(Wrapper(&()))
    }
}

// ---------------------------------------- //

trait Trait<'a> {
    type Assoc;
    fn new() -> Self::Assoc;
}
impl Trait<'_> for () {
    type Assoc = ();
    fn new() {}
}

impl<'a, T: Trait<'a>> Wrapper<'a, T> {
    async fn mk_assoc() -> T::Assoc {
        //^ Previously rejected because `T::Assoc` doesn't mention `'a` in the HIR,
        //  but ends up resolving to `<T as Trait<'a>>::Assoc`, which does rely on `'a`.
        // That's the important part -- the elided trait.
        T::new()
    }

    fn a_few_assocs() -> impl Iterator<Item = T::Assoc> {
        //^ Previously rejected for the same reason
        [T::new(), T::new(), T::new()].into_iter()
    }
}

// ---------------------------------------- //

trait InTrait {
    async fn async_fn() -> Self;

    fn impl_trait() -> impl Iterator<Item = Self>;
}

impl InTrait for &() {
    async fn async_fn() -> Self { &() }
    //^ Previously rejected just like inherent impls

    fn impl_trait() -> impl Iterator<Item = Self> {
        //^ Previously rejected just like inherent impls
        [&()].into_iter()
    }
}
```

## Technical:

Lifetimes in return-position `impl Trait` (and `async fn`) are duplicated as early-bound generics local to the opaque in order to make sure we are able to substitute any late-bound lifetimes from the function in the opaque's hidden type. (The [dev guide](https://rustc-dev-guide.rust-lang.org/return-position-impl-trait-in-trait.html#aside-opaque-lifetime-duplication) has a small section about why this is necessary -- this was written for RPITITs, but it applies to all RPITs)

Prior to #103491, all of the early-bound lifetimes not local to the opaque were replaced with `'static` to avoid issues where relating opaques caused their *non-captured* lifetimes to be related. This `'static` replacement led to strange and possibly unsound behaviors (https://github.com/rust-lang/rust/issues/61949#issuecomment-508836314) (https://github.com/rust-lang/rust/issues/53613) when referencing the `Self` type alias in an impl or indirectly referencing a lifetime parameter via a projection type (via a `T::Assoc` projection without an explicit trait), since lifetime resolution is performed on the HIR, when neither `T::Assoc`-style projections or `Self` in impls are expanded.

Therefore an error was implemented in #62849 to deny this subtle behavior as a known limitation of the compiler. It was attempted by `@cjgillot` to fix this in #91403, which was subsequently unlanded. Then it was re-attempted to much success (🎉) in #103491, which is where we currently are in the compiler.

The PR above (#103491) fixed this issue technically by *not* replacing the opaque's parent lifetimes with `'static`, but instead using variance to properly track which lifetimes are captured and are not. The PR gated any of the "side-effects" of the PR behind a feature gate (`impl_trait_projections`) presumably to avoid having to involve T-lang or T-types in the PR as well. `@cjgillot` can clarify this if I'm misunderstanding what their intention was with the feature gate.

Since we're not replacing (possibly *invariant*!) lifetimes with `'static` anymore, there are no more soundness concerns here. Therefore, this PR removes the feature gate.

Tests:
* `tests/ui/async-await/feature-self-return-type.rs`
* `tests/ui/impl-trait/feature-self-return-type.rs`
* `tests/ui/async-await/issues/issue-78600.rs`
* `tests/ui/impl-trait/capture-lifetime-not-in-hir.rs`

---

r? cjgillot on the impl (not much, just removing the feature gate)

I'm gonna mark this as FCP for T-lang and T-types.
This commit is contained in:
bors 2023-09-28 21:35:18 +00:00
commit 7b4d9e155f
17 changed files with 50 additions and 226 deletions
Download patch file Download diff file Expand all files Collapse all files

135
compiler/rustc_hir_analysis/src/check/check.rs
View file

@ -5,18 +5,15 @@ use super::compare_impl_item::check_type_bounds;
use super::compare_impl_item::{compare_impl_method, compare_impl_ty};
use super::*;
use rustc_attr as attr;
use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan};
use rustc_errors::{ErrorGuaranteed, MultiSpan};
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
use rustc_hir::intravisit::Visitor;
use rustc_hir::{ItemKind, Node, PathSegment};
use rustc_infer::infer::opaque_types::ConstrainOpaqueTypeRegionVisitor;
use rustc_hir::Node;
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
use rustc_infer::traits::{Obligation, TraitEngineExt as _};
use rustc_lint_defs::builtin::REPR_TRANSPARENT_EXTERNAL_PRIVATE_FIELDS;
use rustc_middle::hir::nested_filter;
use rustc_middle::middle::stability::EvalResult;
use rustc_middle::traits::{DefiningAnchor, ObligationCauseCode};
use rustc_middle::ty::fold::BottomUpFolder;
@ -218,9 +215,6 @@ fn check_opaque(tcx: TyCtxt<'_>, id: hir::ItemId) {
let args = GenericArgs::identity_for_item(tcx, item.owner_id);
let span = tcx.def_span(item.owner_id.def_id);
if !tcx.features().impl_trait_projections {
check_opaque_for_inheriting_lifetimes(tcx, item.owner_id.def_id, span);
}
if tcx.type_of(item.owner_id.def_id).instantiate_identity().references_error() {
return;
}
@ -231,129 +225,6 @@ fn check_opaque(tcx: TyCtxt<'_>, id: hir::ItemId) {
let _ = check_opaque_meets_bounds(tcx, item.owner_id.def_id, span, &origin);
}
/// Checks that an opaque type does not use `Self` or `T::Foo` projections that would result
/// in "inheriting lifetimes".
#[instrument(level = "debug", skip(tcx, span))]
pub(super) fn check_opaque_for_inheriting_lifetimes(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
span: Span,
) {
let item = tcx.hir().expect_item(def_id);
debug!(?item, ?span);
struct ProhibitOpaqueVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
opaque_identity_ty: Ty<'tcx>,
parent_count: u32,
references_parent_regions: bool,
selftys: Vec<(Span, Option<String>)>,
}
impl<'tcx> ty::visit::TypeVisitor<TyCtxt<'tcx>> for ProhibitOpaqueVisitor<'tcx> {
type BreakTy = Ty<'tcx>;
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
debug!(?t, "root_visit_ty");
if t == self.opaque_identity_ty {
ControlFlow::Continue(())
} else {
t.visit_with(&mut ConstrainOpaqueTypeRegionVisitor {
tcx: self.tcx,
op: |region| {
if let ty::ReEarlyBound(ty::EarlyBoundRegion { index, .. }) = *region
&& index < self.parent_count
{
self.references_parent_regions= true;
}
},
});
if self.references_parent_regions {
ControlFlow::Break(t)
} else {
ControlFlow::Continue(())
}
}
}
}
impl<'tcx> Visitor<'tcx> for ProhibitOpaqueVisitor<'tcx> {
type NestedFilter = nested_filter::OnlyBodies;
fn nested_visit_map(&mut self) -> Self::Map {
self.tcx.hir()
}
fn visit_ty(&mut self, arg: &'tcx hir::Ty<'tcx>) {
match arg.kind {
hir::TyKind::Path(hir::QPath::Resolved(None, path)) => match &path.segments {
[PathSegment { res: Res::SelfTyParam { .. }, .. }] => {
let impl_ty_name = None;
self.selftys.push((path.span, impl_ty_name));
}
[PathSegment { res: Res::SelfTyAlias { alias_to: def_id, .. }, .. }] => {
let impl_ty_name = Some(self.tcx.def_path_str(*def_id));
self.selftys.push((path.span, impl_ty_name));
}
_ => {}
},
_ => {}
}
hir::intravisit::walk_ty(self, arg);
}
}
if let ItemKind::OpaqueTy(&hir::OpaqueTy {
origin: hir::OpaqueTyOrigin::AsyncFn(..) | hir::OpaqueTyOrigin::FnReturn(..),
..
}) = item.kind
{
let args = GenericArgs::identity_for_item(tcx, def_id);
let opaque_identity_ty = Ty::new_opaque(tcx, def_id.to_def_id(), args);
let mut visitor = ProhibitOpaqueVisitor {
opaque_identity_ty,
parent_count: tcx.generics_of(def_id).parent_count as u32,
references_parent_regions: false,
tcx,
selftys: vec![],
};
let prohibit_opaque = tcx
.explicit_item_bounds(def_id)
.instantiate_identity_iter_copied()
.try_for_each(|(predicate, _)| predicate.visit_with(&mut visitor));
if let Some(ty) = prohibit_opaque.break_value() {
visitor.visit_item(&item);
let is_async = match item.kind {
ItemKind::OpaqueTy(hir::OpaqueTy { origin, .. }) => {
matches!(origin, hir::OpaqueTyOrigin::AsyncFn(..))
}
_ => unreachable!(),
};
let mut err = feature_err(
&tcx.sess.parse_sess,
sym::impl_trait_projections,
span,
format!(
"`{}` return type cannot contain a projection or `Self` that references \
lifetimes from a parent scope",
if is_async { "async fn" } else { "impl Trait" },
),
);
for (span, name) in visitor.selftys {
err.span_suggestion(
span,
"consider spelling out the type instead",
name.unwrap_or_else(|| format!("{ty:?}")),
Applicability::MaybeIncorrect,
);
}
err.emit();
}
}
}
/// Checks that an opaque type does not contain cycles.
pub(super) fn check_opaque_for_cycles<'tcx>(
tcx: TyCtxt<'tcx>,