Skip to content

Commit

Permalink
Auto merge of #112652 - oli-obk:tait_only_in_sig, r=compiler-errors
Browse files Browse the repository at this point in the history
Require TAITs to be mentioned in the signatures of functions that register hidden types for them

r? `@lcnr` `@compiler-errors`

This implements the lang team decision from [the TAIT design meeting](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/design.20meeting.202023-05-31.20TAITs/near/362518164).
  • Loading branch information
bors committed Jul 8, 2023
2 parents eee6b31 + c3004a7 commit d4096e0
Show file tree
Hide file tree
Showing 76 changed files with 781 additions and 189 deletions.
3 changes: 1 addition & 2 deletions compiler/rustc_error_messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ fn register_functions(bundle: &mut FluentBundle) {
pub type LazyFallbackBundle = Lrc<Lazy<FluentBundle, impl FnOnce() -> FluentBundle>>;

/// Return the default `FluentBundle` with standard "en-US" diagnostic messages.
#[instrument(level = "trace")]
#[instrument(level = "trace", skip(resources))]
pub fn fallback_fluent_bundle(
resources: Vec<&'static str>,
with_directionality_markers: bool,
Expand All @@ -242,7 +242,6 @@ pub fn fallback_fluent_bundle(
for resource in resources {
let resource = FluentResource::try_new(resource.to_string())
.expect("failed to parse fallback fluent resource");
trace!(?resource);
fallback_bundle.add_resource_overriding(resource);
}

Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ hir_analysis_static_specialize = cannot specialize on `'static` lifetime
hir_analysis_substs_on_overridden_impl = could not resolve substs on overridden impl
hir_analysis_tait_forward_compat = item constrains opaque type that is not in its signature
.note = this item must mention the opaque type in its signature in order to be able to register hidden types
hir_analysis_target_feature_on_main = `main` function is not allowed to have `#[target_feature]`
hir_analysis_too_large_static = extern static is too large for the current architecture
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_middle::hir::nested_filter;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
use rustc_span::DUMMY_SP;

use crate::errors::UnconstrainedOpaqueType;
use crate::errors::{TaitForwardCompat, UnconstrainedOpaqueType};

/// Checks "defining uses" of opaque `impl Trait` types to ensure that they meet the restrictions
/// laid for "higher-order pattern unification".
Expand Down Expand Up @@ -139,6 +139,15 @@ impl TaitConstraintLocator<'_> {
continue;
}
constrained = true;
if !self.tcx.opaque_types_defined_by(item_def_id).contains(&self.def_id) {
self.tcx.sess.emit_err(TaitForwardCompat {
span: hidden_type.span,
item_span: self
.tcx
.def_ident_span(item_def_id)
.unwrap_or_else(|| self.tcx.def_span(item_def_id)),
});
}
let concrete_type =
self.tcx.erase_regions(hidden_type.remap_generic_params_to_declaration_params(
opaque_type_key,
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,16 @@ pub struct UnconstrainedOpaqueType {
pub what: &'static str,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_tait_forward_compat)]
#[note]
pub struct TaitForwardCompat {
#[primary_span]
pub span: Span,
#[note]
pub item_span: Span,
}

pub struct MissingTypeParams {
pub span: Span,
pub def_span: Span,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/inherited.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl<'tcx> Inherited<'tcx> {
let infcx = tcx
.infer_ctxt()
.ignoring_regions()
.with_opaque_type_inference(DefiningAnchor::Bind(hir_owner.def_id))
.with_opaque_type_inference(DefiningAnchor::Bind(def_id))
.build();
let typeck_results = RefCell::new(ty::TypeckResults::new(hir_owner));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ impl<T> Trait<T> for X {
);
}
}
(ty::Alias(ty::Opaque, alias), _) | (_, ty::Alias(ty::Opaque, alias)) if alias.def_id.is_local() && matches!(tcx.def_kind(body_owner_def_id), DefKind::AssocFn | DefKind::AssocConst) => {
(ty::Alias(ty::Opaque, alias), _) | (_, ty::Alias(ty::Opaque, alias)) if alias.def_id.is_local() && matches!(tcx.def_kind(body_owner_def_id), DefKind::Fn | DefKind::Static(_) | DefKind::Const | DefKind::AssocFn | DefKind::AssocConst) => {
if tcx.is_type_alias_impl_trait(alias.def_id) {
if !tcx.opaque_types_defined_by(body_owner_def_id.expect_local()).contains(&alias.def_id.expect_local()) {
let sp = tcx.def_ident_span(body_owner_def_id).unwrap_or_else(|| tcx.def_span(body_owner_def_id));
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mod fulfill;
pub mod misc;
mod object_safety;
pub mod outlives_bounds;
mod project;
pub mod project;
pub mod query;
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
mod select;
Expand Down
152 changes: 117 additions & 35 deletions compiler/rustc_ty_utils/src/opaque_types.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use rustc_data_structures::fx::FxHashSet;
use rustc_hir::intravisit::Visitor;
use rustc_hir::{def::DefKind, def_id::LocalDefId};
use rustc_hir::{intravisit, CRATE_HIR_ID};
use rustc_middle::query::Providers;
use rustc_middle::ty::util::{CheckRegions, NotUniqueParam};
use rustc_middle::ty::{self, Ty, TyCtxt};
Expand Down Expand Up @@ -51,7 +53,7 @@ impl<'tcx> OpaqueTypeCollector<'tcx> {

fn parent(&self) -> Option<LocalDefId> {
match self.tcx.def_kind(self.item) {
DefKind::Fn => None,
DefKind::AnonConst | DefKind::InlineConst | DefKind::Fn | DefKind::TyAlias => None,
DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst => {
Some(self.tcx.local_parent(self.item))
}
Expand All @@ -61,6 +63,73 @@ impl<'tcx> OpaqueTypeCollector<'tcx> {
),
}
}

/// Returns `true` if `opaque_hir_id` is a sibling or a child of a sibling of `self.item`.
///
/// Example:
/// ```ignore UNSOLVED (is this a bug?)
/// # #![feature(type_alias_impl_trait)]
/// pub mod foo {
/// pub mod bar {
/// pub trait Bar { /* ... */ }
/// pub type Baz = impl Bar;
///
/// # impl Bar for () {}
/// fn f1() -> Baz { /* ... */ }
/// }
/// fn f2() -> bar::Baz { /* ... */ }
/// }
/// ```
///
/// and `opaque_def_id` is the `DefId` of the definition of the opaque type `Baz`.
/// For the above example, this function returns `true` for `f1` and `false` for `f2`.
#[instrument(level = "trace", skip(self), ret)]
fn check_tait_defining_scope(&self, opaque_def_id: LocalDefId) -> bool {
let mut hir_id = self.tcx.hir().local_def_id_to_hir_id(self.item);
let opaque_hir_id = self.tcx.hir().local_def_id_to_hir_id(opaque_def_id);

// Named opaque types can be defined by any siblings or children of siblings.
let scope = self.tcx.hir().get_defining_scope(opaque_hir_id);
// We walk up the node tree until we hit the root or the scope of the opaque type.
while hir_id != scope && hir_id != CRATE_HIR_ID {
hir_id = self.tcx.hir().get_parent_item(hir_id).into();
}
// Syntactically, we are allowed to define the concrete type if:
hir_id == scope
}

fn collect_body_and_predicate_taits(&mut self) {
// Look at all where bounds.
self.tcx.predicates_of(self.item).instantiate_identity(self.tcx).visit_with(self);
// An item is allowed to constrain opaques declared within its own body (but not nested within
// nested functions).
self.collect_taits_declared_in_body();
}

#[instrument(level = "trace", skip(self))]
fn collect_taits_declared_in_body(&mut self) {
let body = self.tcx.hir().body(self.tcx.hir().body_owned_by(self.item)).value;
struct TaitInBodyFinder<'a, 'tcx> {
collector: &'a mut OpaqueTypeCollector<'tcx>,
}
impl<'v> intravisit::Visitor<'v> for TaitInBodyFinder<'_, '_> {
#[instrument(level = "trace", skip(self))]
fn visit_nested_item(&mut self, id: rustc_hir::ItemId) {
let id = id.owner_id.def_id;
if let DefKind::TyAlias = self.collector.tcx.def_kind(id) {
let items = self.collector.tcx.opaque_types_defined_by(id);
self.collector.opaques.extend(items);
}
}
#[instrument(level = "trace", skip(self))]
// Recurse into these, as they are type checked with their parent
fn visit_nested_body(&mut self, id: rustc_hir::BodyId) {
let body = self.collector.tcx.hir().body(id);
self.visit_body(body);
}
}
TaitInBodyFinder { collector: self }.visit_expr(body);
}
}

impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
Expand All @@ -73,6 +142,21 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
return ControlFlow::Continue(());
}

// TAITs outside their defining scopes are ignored.
let origin = self.tcx.opaque_type_origin(alias_ty.def_id.expect_local());
trace!(?origin);
match origin {
rustc_hir::OpaqueTyOrigin::FnReturn(_)
| rustc_hir::OpaqueTyOrigin::AsyncFn(_) => {}
rustc_hir::OpaqueTyOrigin::TyAlias { in_assoc_ty } => {
if !in_assoc_ty {
if !self.check_tait_defining_scope(alias_ty.def_id.expect_local()) {
return ControlFlow::Continue(());
}
}
}
}

self.opaques.push(alias_ty.def_id.expect_local());

match self.tcx.uses_unique_generic_params(alias_ty.substs, CheckRegions::Bound) {
Expand Down Expand Up @@ -188,65 +272,63 @@ impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for OpaqueTypeCollector<'tcx> {
fn opaque_types_defined_by<'tcx>(tcx: TyCtxt<'tcx>, item: LocalDefId) -> &'tcx [LocalDefId] {
let kind = tcx.def_kind(item);
trace!(?kind);
// FIXME(type_alias_impl_trait): This is definitely still wrong except for RPIT and impl trait in assoc types.
let mut collector = OpaqueTypeCollector::new(tcx, item);
match kind {
// Walk over the signature of the function-like to find the opaques.
DefKind::AssocFn | DefKind::Fn => {
let ty_sig = tcx.fn_sig(item).subst_identity();
let hir_sig = tcx.hir().get_by_def_id(item).fn_sig().unwrap();
// Walk over the inputs and outputs manually in order to get good spans for them.
collector.visit_spanned(hir_sig.decl.output.span(), ty_sig.output());
for (hir, ty) in hir_sig.decl.inputs.iter().zip(ty_sig.inputs().iter()) {
collector.visit_spanned(hir.span, ty.map_bound(|x| *x));
}
collector.collect_body_and_predicate_taits();
}
// Walk over the type of the item to find opaques.
DefKind::Static(_) | DefKind::Const | DefKind::AssocConst | DefKind::AnonConst => {
let span = match tcx.hir().get_by_def_id(item).ty() {
Some(ty) => ty.span,
_ => tcx.def_span(item),
};
collector.visit_spanned(span, tcx.type_of(item).subst_identity());
collector.collect_body_and_predicate_taits();
}
// We're also doing this for `AssocTy` for the wf checks in `check_opaque_meets_bounds`
DefKind::Fn | DefKind::AssocFn | DefKind::AssocTy | DefKind::AssocConst => {
let mut collector = OpaqueTypeCollector::new(tcx, item);
match kind {
// Walk over the signature of the function-like to find the opaques.
DefKind::AssocFn | DefKind::Fn => {
let ty_sig = tcx.fn_sig(item).subst_identity();
let hir_sig = tcx.hir().get_by_def_id(item).fn_sig().unwrap();
// Walk over the inputs and outputs manually in order to get good spans for them.
collector.visit_spanned(hir_sig.decl.output.span(), ty_sig.output());
for (hir, ty) in hir_sig.decl.inputs.iter().zip(ty_sig.inputs().iter()) {
collector.visit_spanned(hir.span, ty.map_bound(|x| *x));
}
}
// Walk over the type of the item to find opaques.
DefKind::AssocTy | DefKind::AssocConst => {
let span = match tcx.hir().get_by_def_id(item).ty() {
Some(ty) => ty.span,
_ => tcx.def_span(item),
};
collector.visit_spanned(span, tcx.type_of(item).subst_identity());
}
_ => unreachable!(),
DefKind::TyAlias | DefKind::AssocTy => {
tcx.type_of(item).subst_identity().visit_with(&mut collector);
}
DefKind::OpaqueTy => {
for (pred, span) in tcx.explicit_item_bounds(item).subst_identity_iter_copied() {
collector.visit_spanned(span, pred);
}
tcx.arena.alloc_from_iter(collector.opaques)
}
DefKind::Mod
| DefKind::Struct
| DefKind::Union
| DefKind::Enum
| DefKind::Variant
| DefKind::Trait
| DefKind::TyAlias
| DefKind::ForeignTy
| DefKind::TraitAlias
| DefKind::TyParam
| DefKind::Const
| DefKind::ConstParam
| DefKind::Static(_)
| DefKind::Ctor(_, _)
| DefKind::Macro(_)
| DefKind::ExternCrate
| DefKind::Use
| DefKind::ForeignMod
| DefKind::AnonConst
| DefKind::InlineConst
| DefKind::OpaqueTy
| DefKind::ImplTraitPlaceholder
| DefKind::Field
| DefKind::LifetimeParam
| DefKind::GlobalAsm
| DefKind::Impl { .. }
| DefKind::Closure
| DefKind::Generator => {
span_bug!(tcx.def_span(item), "{kind:?} is type checked as part of its parent")
| DefKind::Impl { .. } => {}
// Closures and generators are type checked with their parent, so there is no difference here.
DefKind::Closure | DefKind::Generator | DefKind::InlineConst => {
return tcx.opaque_types_defined_by(tcx.local_parent(item));
}
}
tcx.arena.alloc_from_iter(collector.opaques)
}

pub(super) fn provide(providers: &mut Providers) {
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/feature-gates/feature-gate-type_alias_impl_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn define() -> Bar {

type Foo2 = impl Debug;

fn define2() {
fn define2(_: Foo2) {
let x = || -> Foo2 { 42 };
}

Expand All @@ -20,13 +20,13 @@ type Foo3 = impl Debug;
fn define3(x: Foo3) {
let y: i32 = x;
}
fn define3_1() {
fn define3_1(_: Foo3) {
define3(42)
}

type Foo4 = impl Debug;

fn define4() {
fn define4(_: Foo4) {
let y: Foo4 = 42;
}

Expand Down
23 changes: 23 additions & 0 deletions tests/ui/generic-associated-types/issue-90014-tait.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//! This test is reporting the wrong error. We need
//! more inherent associated type tests that use opaque types
//! in general. Some variant of this test should compile successfully.
// known-bug: unknown
// edition:2018

#![feature(impl_trait_in_assoc_type, inherent_associated_types)]
#![allow(incomplete_features)]

use std::future::Future;

struct Foo<'a>(&'a mut ());

impl Foo<'_> {
type Fut<'a> = impl Future<Output = ()>;
//^ ERROR: the type `&mut ()` does not fulfill the required lifetime

fn make_fut<'a>(&'a self) -> Self::Fut<'a> {
async { () }
}
}

fn main() {}
22 changes: 22 additions & 0 deletions tests/ui/generic-associated-types/issue-90014-tait.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0308]: mismatched types
--> $DIR/issue-90014-tait.rs:19:9
|
LL | type Fut<'a> = impl Future<Output = ()>;
| ------------------------ the expected future
...
LL | fn make_fut<'a>(&'a self) -> Self::Fut<'a> {
| ------------- expected `Foo<'_>::Fut<'a>` because of return type
LL | async { () }
| ^^^^^^^^^^^^ expected future, found `async` block
|
= note: expected opaque type `Foo<'_>::Fut<'a>`
found `async` block `[async block@$DIR/issue-90014-tait.rs:19:9: 19:21]`
note: this item must have the opaque type in its signature in order to be able to register hidden types
--> $DIR/issue-90014-tait.rs:18:8
|
LL | fn make_fut<'a>(&'a self) -> Self::Fut<'a> {
| ^^^^^^^^

error: aborting due to previous error

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

0 comments on commit d4096e0

Please sign in to comment.