Skip to content

Require TAITs to appear in the signature of items that register a hidden type #107809

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

Closed
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
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,8 @@ declare_features! (
(active, try_blocks, "1.29.0", Some(31436), None),
/// Allows `impl Trait` to be used inside type aliases (RFC 2515).
(active, type_alias_impl_trait, "1.38.0", Some(63063), None),
/// Allows functions with TAITs in their where bounds to register hidden types.
(active, type_alias_impl_trait_in_where_bounds, "CURRENT_RUSTC_VERSION", Some(63063), None),
/// Allows the use of type ascription in expressions.
(active, type_ascription, "1.6.0", Some(23416), None),
/// Allows creation of instances of a struct by moving fields that have
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ hir_analysis_trait_object_declared_with_no_traits =
at least one trait is required for an object type
.alias_span = this alias does not contain a trait

hir_analysis_opaque_type_constrained_but_not_in_sig = opaque type constrained without being represented in the signature
.item_label = this item must mention the opaque type in its signature or where bounds

hir_analysis_opaque_type_constrained_but_not_in_sibling = opaque type constrained in nested item
.item_label = this item is not a sibling of the opaque type

hir_analysis_missing_type_params =
the type {$parameterCount ->
[one] parameter
Expand Down
210 changes: 202 additions & 8 deletions compiler/rustc_hir_analysis/src/collect/type_of.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use std::ops::ControlFlow;

use hir::def::DefKind;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, StashKey};
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
Expand All @@ -9,15 +13,18 @@ use rustc_middle::ty::print::with_forced_trimmed_paths;
use rustc_middle::ty::subst::InternalSubsts;
use rustc_middle::ty::util::IntTypeExt;
use rustc_middle::ty::{
self, ImplTraitInTraitData, IsSuggestable, Ty, TyCtxt, TypeFolder, TypeSuperFoldable,
TypeVisitableExt,
self, ImplTraitInTraitData, IsSuggestable, Ty, TyCtxt, TypeFoldable, TypeFolder,
TypeSuperFoldable, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor,
};
use rustc_session::parse::feature_err;
use rustc_span::symbol::Ident;
use rustc_span::{Span, DUMMY_SP};
use rustc_span::{sym, Span, DUMMY_SP};

use super::ItemCtxt;
use super::{bad_placeholder, is_suggestable_infer_ty};
use crate::errors::UnconstrainedOpaqueType;
use crate::errors::{
OpaqueTypeConstrainedButNotInSig, OpaqueTypeConstrainedInNonSibling, UnconstrainedOpaqueType,
};

/// Computes the relevant generic parameter for a potential generic const argument.
///
Expand Down Expand Up @@ -635,6 +642,21 @@ fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> T
let concrete_opaque_types = &self.tcx.mir_borrowck(item_def_id).concrete_opaque_types;
debug!(?concrete_opaque_types);
if let Some(&concrete_type) = concrete_opaque_types.get(&self.def_id) {
if let Err((item_def_id, sibling)) =
may_define_opaque_type(self.tcx, item_def_id, self.def_id, concrete_type.span)
{
if sibling {
self.tcx.sess.emit_err(OpaqueTypeConstrainedButNotInSig {
span: concrete_type.span,
item_span: self.tcx.def_span(item_def_id),
});
} else {
self.tcx.sess.emit_err(OpaqueTypeConstrainedInNonSibling {
span: concrete_type.span,
item_span: self.tcx.def_span(item_def_id),
});
}
}
debug!(?concrete_type, "found constraint");
if let Some(prev) = &mut self.found {
if concrete_type.ty != prev.ty && !(concrete_type, prev.ty).references_error() {
Expand All @@ -655,8 +677,12 @@ fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> T
self.tcx.hir()
}
fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
if let hir::ExprKind::Closure(closure) = ex.kind {
self.check(closure.def_id);
match ex.kind {
hir::ExprKind::Closure(closure) => {
self.check(closure.def_id);
}
hir::ExprKind::ConstBlock(anon) => self.check(anon.def_id),
_ => (),
}
intravisit::walk_expr(self, ex);
}
Expand Down Expand Up @@ -743,6 +769,176 @@ fn find_opaque_ty_constraints_for_tait(tcx: TyCtxt<'_>, def_id: LocalDefId) -> T
hidden.ty
}

#[instrument(skip(tcx), level = "trace", ret)]
fn may_define_opaque_type<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
opaque_def_id: LocalDefId,
span: Span,
) -> Result<(), (LocalDefId, bool)> {
let mut parent = tcx.local_parent(opaque_def_id);
loop {
trace!(?parent);
match tcx.def_kind(parent) {
DefKind::AssocTy | DefKind::TyAlias => {
parent = tcx.local_parent(parent);
break;
}
// Skip nested opaque types
DefKind::OpaqueTy => {
parent = tcx.local_parent(parent);
}
def_kind => {
trace!(?def_kind);
return Err((def_id, false));
}
}
}
trace!(?parent);
if parent == def_id {
// If the opaque type is defined in the body of a function, that function
// may constrain the opaque type since it can't mention it in bounds.
return Ok(());
}

if tcx.is_typeck_child(def_id.to_def_id()) {
return may_define_opaque_type(tcx, tcx.local_parent(def_id), opaque_def_id, span);
}

let mut item_parent = tcx.local_parent(def_id);
while item_parent != parent {
trace!(?item_parent);
match tcx.def_kind(item_parent) {
// Skip impls, to allow methods to constrain opaque types from the surrounding module.
DefKind::Impl { .. } | DefKind::Trait => {
item_parent = tcx.local_parent(item_parent);
}
// Skip modules, to allow constraining opaque types in child modules
DefKind::Mod => {
item_parent = tcx.local_parent(item_parent);
}
def_kind => {
trace!(?def_kind);
break;
}
}
}

if item_parent != parent {
return Err((def_id, false));
}

let param_env = tcx.param_env(def_id);

fn has_tait<'tcx>(
val: impl TypeVisitable<TyCtxt<'tcx>>,
opaque_def_id: LocalDefId,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> bool {
struct Visitor<'tcx> {
opaque_def_id: DefId,
tcx: TyCtxt<'tcx>,
seen: FxHashSet<Ty<'tcx>>,
param_env: ty::ParamEnv<'tcx>,
}
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for Visitor<'tcx> {
type BreakTy = ();

#[instrument(skip(self), level = "trace", ret)]
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
// Erase all lifetimes, they can't affect anything, but recursion
// may cause late bound regions to differ, because the type visitor
// can't erase them in `visit_binder`.
let t = t.fold_with(&mut ty::fold::BottomUpFolder {
tcx: self.tcx,
ct_op: |c| c,
ty_op: |t| t,
lt_op: |_| self.tcx.lifetimes.re_erased,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't erase bound regions here, only escaping bound regions 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could implement fn fold_binder to call https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html#method.replace_bound_vars_uncached, replacing bound vars with self.tcx.lifetime.re_erased and eagerly replace all free regions that way

});
if !self.seen.insert(t) {
return ControlFlow::Continue(());
}
match t.kind() {
ty::Alias(ty::Opaque, alias) => {
if alias.def_id == self.opaque_def_id {
return ControlFlow::Break(());
}
for (pred, _span) in self
.tcx
.bound_explicit_item_bounds(alias.def_id)
.subst_iter_copied(self.tcx, alias.substs)
{
pred.visit_with(self)?;
}
}
ty::Alias(ty::Projection, _) => {
if let Ok(proj) = self.tcx.try_normalize_erasing_regions(self.param_env, t)
{
proj.visit_with(self)?;
}
}
// Types that have opaque type fields must get walked manually, they
// would not be seen by the type visitor otherwise.
ty::Adt(adt_def, substs) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the type can be contained in an Adt? please add that to the PR description.

also, this can be adt_def.all_fields instead of the variants -> fields nested loop

for variant in adt_def.variants() {
for field in &variant.fields {
field.ty(self.tcx, substs).visit_with(self)?;
}
}
}
_ => (),
}
t.super_visit_with(self)
}
}
val.visit_with(&mut Visitor {
opaque_def_id: opaque_def_id.to_def_id(),
tcx,
seen: Default::default(),
param_env,
})
.is_break()
}
let tait_in_fn_sig = match tcx.def_kind(def_id) {
DefKind::AssocFn | DefKind::Fn => {
has_tait(tcx.fn_sig(def_id.to_def_id()).skip_binder(), opaque_def_id, tcx, param_env)
}
// Opaque types in types of contsts
DefKind::Static(_) | DefKind::Const | DefKind::AssocConst => {
has_tait(tcx.type_of(def_id.to_def_id()).skip_binder(), opaque_def_id, tcx, param_env)
}
// Nested opaque types
DefKind::OpaqueTy => has_tait(
tcx.bound_explicit_item_bounds(def_id.to_def_id()).skip_binder(),
opaque_def_id,
tcx,
param_env,
),
_ => false,
};
trace!(?tait_in_fn_sig);
if tait_in_fn_sig {
return Ok(());
}
let tait_in_where_bounds =
has_tait(tcx.predicates_of(def_id.to_def_id()).predicates, opaque_def_id, tcx, param_env);
if tcx.features().type_alias_impl_trait_in_where_bounds {
if tait_in_where_bounds { Ok(()) } else { Err((def_id, true)) }
} else {
if tait_in_where_bounds {
feature_err(
&tcx.sess.parse_sess,
sym::type_alias_impl_trait_in_where_bounds,
span,
"type alias impl trait is only usable in argument or return types",
)
.emit();
}
Err((def_id, true))
}
}

fn find_opaque_ty_constraints_for_rpit(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
Expand Down Expand Up @@ -951,8 +1147,6 @@ fn infer_placeholder_type<'a>(

fn check_feature_inherent_assoc_ty(tcx: TyCtxt<'_>, span: Span) {
if !tcx.features().inherent_associated_types {
use rustc_session::parse::feature_err;
use rustc_span::symbol::sym;
feature_err(
&tcx.sess.parse_sess,
sym::inherent_associated_types,
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,23 @@ pub struct UnconstrainedOpaqueType {
pub what: &'static str,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_opaque_type_constrained_but_not_in_sig)]
pub struct OpaqueTypeConstrainedButNotInSig {
#[primary_span]
pub span: Span,
#[label(hir_analysis_item_label)]
pub item_span: Span,
}
#[derive(Diagnostic)]
#[diag(hir_analysis_opaque_type_constrained_but_not_in_sibling)]
pub struct OpaqueTypeConstrainedInNonSibling {
#[primary_span]
pub span: Span,
#[label(hir_analysis_item_label)]
pub item_span: Span,
}

pub struct MissingTypeParams {
pub span: Span,
pub def_span: Span,
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_hir_typeck/src/inherited.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,17 @@ impl<'tcx> Deref for Inherited<'tcx> {
}

impl<'tcx> Inherited<'tcx> {
pub fn new(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> Self {
pub fn new(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
mk_defining_use_anchor: impl FnOnce(LocalDefId) -> DefiningAnchor,
) -> Self {
let hir_owner = tcx.hir().local_def_id_to_hir_id(def_id).owner;

let infcx = tcx
.infer_ctxt()
.ignoring_regions()
.with_opaque_type_inference(DefiningAnchor::Bind(hir_owner.def_id))
.with_opaque_type_inference(mk_defining_use_anchor(hir_owner.def_id))
.build();
let typeck_results = RefCell::new(ty::TypeckResults::new(hir_owner));

Expand Down
15 changes: 13 additions & 2 deletions compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ mod writeback;

pub use fn_ctxt::FnCtxt;
pub use inherited::Inherited;
use rustc_infer::infer::DefiningAnchor;

use crate::check::check_fn;
use crate::coercion::DynamicCoerceMany;
Expand Down Expand Up @@ -212,11 +213,21 @@ fn typeck_with_fallback<'tcx>(
} else {
param_env
};
let inh = Inherited::new(tcx, def_id);
let fn_sig_infer = fn_sig.map_or(false, |fn_sig| {
rustc_hir_analysis::collect::get_infer_ret_ty(&fn_sig.decl.output).is_some()
});

let mk_defining_use_anchor = |def_id| {
// In case we are inferring the return signature (via `_` types), ignore defining use
// rules, as we'll error out anyway. This helps improve diagnostics, which otherwise
// may just see `ty::Error` instead of `ty::Alias(Opaque, _)` and not produce better errors.
if fn_sig_infer { DefiningAnchor::Bubble } else { DefiningAnchor::Bind(def_id) }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which are the diagnostics changed by this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example is

type Pointer<T> = impl std::ops::Deref<Target=T>;

fn test() -> Pointer<_> {
    //~^ ERROR: the placeholder `_` is not allowed within types
    Box::new(1)
}

which produces the following error:

error[E0121]: the placeholder `_` is not allowed within types on item signatures for return types
  --> $DIR/issue-77179.rs:7:22
   |
LL | fn test() -> Pointer<_> {
   |              --------^-
   |              |       |
   |              |       not allowed in type signatures
   |              help: replace with the correct return type: `Pointer<i32>`

but without the change it would not see Pointer<i32> but just TyKind::Error and thus not report any suggestion, even if the main error would still be emitted

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't using Pointer<i32> forbidden here anyways, it currently results in the following (bad) error: playground

error[E0792]: expected generic type parameter, found `i32`
 --> src/lib.rs:5:5
  |
2 | type Pointer<T> = impl std::ops::Deref<Target = T>;
  |              - this generic parameter must be used with a generic type parameter
...
5 |     Box::new(1)
  |     ^^^^^^^^^^^

i personally feel like this is not worth it as it imo makes the code noticeably more confusing for a fairly small benefit.

};
let inh = Inherited::new(tcx, def_id, mk_defining_use_anchor);
let mut fcx = FnCtxt::new(&inh, param_env, def_id);

if let Some(hir::FnSig { header, decl, .. }) = fn_sig {
let fn_sig = if rustc_hir_analysis::collect::get_infer_ret_ty(&decl.output).is_some() {
let fn_sig = if fn_sig_infer {
fcx.astconv().ty_of_fn(id, header.unsafety, header.abi, decl, None, None)
} else {
tcx.fn_sig(def_id).subst_identity()
Expand Down
13 changes: 8 additions & 5 deletions compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,6 @@ impl<'tcx> InferCtxt<'tcx> {
/// in its defining scope.
#[instrument(skip(self), level = "trace", ret)]
pub fn opaque_type_origin(&self, def_id: LocalDefId) -> Option<OpaqueTyOrigin> {
let opaque_hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id);
let parent_def_id = match self.defining_use_anchor {
DefiningAnchor::Bubble | DefiningAnchor::Error => return None,
DefiningAnchor::Bind(bind) => bind,
Expand All @@ -385,9 +384,7 @@ impl<'tcx> InferCtxt<'tcx> {
// Anonymous `impl Trait`
hir::OpaqueTyOrigin::FnReturn(parent) => parent == parent_def_id,
// Named `type Foo = impl Bar;`
hir::OpaqueTyOrigin::TyAlias => {
may_define_opaque_type(self.tcx, parent_def_id, opaque_hir_id)
}
hir::OpaqueTyOrigin::TyAlias => may_define_opaque_type(self.tcx, parent_def_id, def_id),
};
in_definition_scope.then_some(origin)
}
Expand Down Expand Up @@ -634,7 +631,13 @@ impl<'tcx> InferCtxt<'tcx> {
/// Here, `def_id` is the `LocalDefId` of the defining use of the opaque type (e.g., `f1` or `f2`),
/// and `opaque_hir_id` is the `HirId` of the definition of the opaque type `Baz`.
/// For the above example, this function returns `true` for `f1` and `false` for `f2`.
fn may_define_opaque_type(tcx: TyCtxt<'_>, def_id: LocalDefId, opaque_hir_id: hir::HirId) -> bool {
#[instrument(skip(tcx), level = "trace", ret)]
fn may_define_opaque_type<'tcx>(
tcx: TyCtxt<'tcx>,
def_id: LocalDefId,
opaque_def_id: LocalDefId,
) -> bool {
let opaque_hir_id = tcx.hir().local_def_id_to_hir_id(opaque_def_id);
let mut hir_id = tcx.hir().local_def_id_to_hir_id(def_id);

// Named opaque types can be defined by any siblings or children of siblings.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/normalize_erasing_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ impl<'tcx> TryNormalizeAfterErasingRegionsFolder<'tcx> {
TryNormalizeAfterErasingRegionsFolder { tcx, param_env }
}

#[instrument(skip(self), level = "debug")]
#[instrument(skip(self), level = "debug", ret)]
fn try_normalize_generic_arg_after_erasing_regions(
&self,
arg: ty::GenericArg<'tcx>,
Expand Down
Loading