Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 3 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ 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_bug_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_missing_type_params =
the type {$parameterCount ->
[one] parameter
Expand Down
127 changes: 124 additions & 3 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,15 @@ 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_span::symbol::Ident;
use rustc_span::{Span, DUMMY_SP};

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

/// Computes the relevant generic parameter for a potential generic const argument.
///
Expand Down Expand Up @@ -635,6 +639,12 @@ 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 !may_define_opaque_type(self.tcx, item_def_id, self.def_id) {
self.tcx.sess.emit_err(OpaqueTypeConstrainedButNotInSig {
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 Down Expand Up @@ -743,6 +753,117 @@ 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,
) -> bool {
if tcx.is_descendant_of(opaque_def_id.to_def_id(), def_id.to_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 true;
}

if tcx.is_closure(def_id.to_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.

Should this only apply for closures, or also for inline consts? IOW, should we check is_typeck_child here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a test for the ban on closure signatures? I saw tests/ui/type-alias-impl-trait/issue-63263-closure-return.rs is changed to accommodate it, but I missed the "fail" test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right on all accounts. There were no tests, and there were especially none for inline consts. I added them and fixed it so that inline consts actually work at all.

return may_define_opaque_type(tcx, tcx.local_parent(def_id), opaque_def_id);
}

let param_env = tcx.param_env(def_id);

trace!(parent = ?tcx.parent(opaque_def_id.to_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);
tait_in_fn_sig
|| has_tait(tcx.predicates_of(def_id.to_def_id()).predicates, opaque_def_id, tcx, param_env)
}

fn find_opaque_ty_constraints_for_rpit(
tcx: TyCtxt<'_>,
def_id: LocalDefId,
Expand Down
9 changes: 9 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,15 @@ pub struct UnconstrainedOpaqueType {
pub what: &'static str,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_opaque_type_constrained_bug_not_in_sig)]
pub struct OpaqueTypeConstrainedButNotInSig {
#[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
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use rustc_middle::ty::subst::{GenericArg, GenericArgKind, SubstsRef};
use rustc_middle::ty::{self, Clause, EarlyBinder, ParamTy, PredicateKind, ProjectionPredicate, TraitPredicate, Ty};
use rustc_span::{sym, Symbol};
use rustc_trait_selection::traits::{query::evaluate_obligation::InferCtxtExt as _, Obligation, ObligationCause};
use rustc_infer::infer::DefiningAnchor;

use super::UNNECESSARY_TO_OWNED;

Expand Down Expand Up @@ -369,7 +370,7 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty<
Node::Item(item) => {
if let ItemKind::Fn(_, _, body_id) = &item.kind
&& let output_ty = return_ty(cx, item.owner_id)
&& let inherited = Inherited::new(cx.tcx, item.owner_id.def_id)
&& let inherited = Inherited::new(cx.tcx, item.owner_id.def_id, DefiningAnchor::Bind)
&& let fn_ctxt = FnCtxt::new(&inherited, cx.param_env, item.owner_id.def_id)
&& fn_ctxt.can_coerce(ty, output_ty)
{
Expand Down
3 changes: 2 additions & 1 deletion src/tools/clippy/clippy_lints/src/transmute/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use rustc_hir_typeck::{cast, FnCtxt, Inherited};
use rustc_lint::LateContext;
use rustc_middle::ty::{cast::CastKind, Ty};
use rustc_span::DUMMY_SP;
use rustc_infer::infer::DefiningAnchor;

// check if the component types of the transmuted collection and the result have different ABI,
// size or alignment
Expand Down Expand Up @@ -33,7 +34,7 @@ pub(super) fn check_cast<'tcx>(
let hir_id = e.hir_id;
let local_def_id = hir_id.owner.def_id;

let inherited = Inherited::new(cx.tcx, local_def_id);
let inherited = Inherited::new(cx.tcx, local_def_id, DefiningAnchor::Bind);
let fn_ctxt = FnCtxt::new(&inherited, cx.param_env, local_def_id);

// If we already have errors, we can't be sure we can pointer cast.
Expand Down
Loading