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

Allow opaque types in trait impl headers and rely on coherence to reject unsound cases #103488

Merged
merged 10 commits into from
Nov 23, 2022
16 changes: 2 additions & 14 deletions compiler/rustc_borrowck/src/type_check/relate_tys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use rustc_infer::infer::nll_relate::{NormalizationStrategy, TypeRelating, TypeRe
use rustc_infer::infer::NllRegionVariableOrigin;
use rustc_infer::traits::PredicateObligations;
use rustc_middle::mir::ConstraintCategory;
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::relate::TypeRelation;
use rustc_middle::ty::{self, Const, Ty};
use rustc_middle::ty::{self, Ty};
use rustc_span::Span;
use rustc_trait_selection::traits::query::Fallible;

Expand Down Expand Up @@ -141,13 +140,6 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx>
);
}

// We don't have to worry about the equality of consts during borrow checking
// as consts always have a static lifetime.
// FIXME(oli-obk): is this really true? We can at least have HKL and with
// inline consts we may have further lifetimes that may be unsound to treat as
// 'static.
fn const_equate(&mut self, _a: Const<'tcx>, _b: Const<'tcx>) {}

fn normalization() -> NormalizationStrategy {
NormalizationStrategy::Eager
}
Expand All @@ -156,10 +148,7 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx>
true
}

fn register_opaque_type_obligations(
&mut self,
obligations: PredicateObligations<'tcx>,
) -> Result<(), TypeError<'tcx>> {
fn register_obligations(&mut self, obligations: PredicateObligations<'tcx>) {
self.type_checker
.fully_perform_op(
self.locations,
Expand All @@ -172,6 +161,5 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for NllTypeRelatingDelegate<'_, '_, 'tcx>
},
)
.unwrap();
Ok(())
}
}
8 changes: 8 additions & 0 deletions compiler/rustc_hir_analysis/src/check/dropck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ impl<'tcx> TypeRelation<'tcx> for SimpleEqRelation<'tcx> {
self.tcx
}

fn intercrate(&self) -> bool {
false
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.param_env
}
Expand All @@ -256,6 +260,10 @@ impl<'tcx> TypeRelation<'tcx> for SimpleEqRelation<'tcx> {
true
}

fn mark_ambiguous(&mut self) {
bug!()
}

fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
_: ty::Variance,
Expand Down
53 changes: 0 additions & 53 deletions compiler/rustc_hir_analysis/src/coherence/orphan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{struct_span_err, DelayDm};
use rustc_errors::{Diagnostic, ErrorGuaranteed};
use rustc_hir as hir;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::subst::InternalSubsts;
use rustc_middle::ty::util::IgnoreRegions;
use rustc_middle::ty::{
Expand Down Expand Up @@ -47,58 +46,6 @@ fn do_orphan_check_impl<'tcx>(
let sp = tcx.def_span(def_id);
let tr = impl_.of_trait.as_ref().unwrap();

// Ensure no opaque types are present in this impl header. See issues #76202 and #86411 for examples,
// and #84660 where it would otherwise allow unsoundness.
if trait_ref.has_opaque_types() {
trace!("{:#?}", item);
// First we find the opaque type in question.
for ty in trait_ref.substs {
for ty in ty.walk() {
let ty::subst::GenericArgKind::Type(ty) = ty.unpack() else { continue };
let ty::Opaque(def_id, _) = *ty.kind() else { continue };
trace!(?def_id);

// Then we search for mentions of the opaque type's type alias in the HIR
struct SpanFinder<'tcx> {
sp: Span,
def_id: DefId,
tcx: TyCtxt<'tcx>,
}
impl<'v, 'tcx> hir::intravisit::Visitor<'v> for SpanFinder<'tcx> {
#[instrument(level = "trace", skip(self, _id))]
fn visit_path(&mut self, path: &'v hir::Path<'v>, _id: hir::HirId) {
// You can't mention an opaque type directly, so we look for type aliases
if let hir::def::Res::Def(hir::def::DefKind::TyAlias, def_id) = path.res {
// And check if that type alias's type contains the opaque type we're looking for
for arg in self.tcx.type_of(def_id).walk() {
if let GenericArgKind::Type(ty) = arg.unpack() {
if let ty::Opaque(def_id, _) = *ty.kind() {
if def_id == self.def_id {
// Finally we update the span to the mention of the type alias
self.sp = path.span;
return;
}
}
}
}
}
hir::intravisit::walk_path(self, path)
}
}

let mut visitor = SpanFinder { sp, def_id, tcx };
hir::intravisit::walk_item(&mut visitor, item);
let reported = tcx
.sess
.struct_span_err(visitor.sp, "cannot implement trait on type alias impl trait")
.span_note(tcx.def_span(def_id), "type alias impl trait defined here")
.emit();
return Err(reported);
}
}
span_bug!(sp, "opaque type not found, but `has_opaque_types` is set")
}

match traits::orphan_check(tcx, item.owner_id.to_def_id()) {
Ok(()) => {}
Err(err) => emit_orphan_check_error(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ fn trait_predicate_kind<'tcx>(
| ty::PredicateKind::ClosureKind(..)
| ty::PredicateKind::ConstEvaluatable(..)
| ty::PredicateKind::ConstEquate(..)
| ty::PredicateKind::Ambiguous
| ty::PredicateKind::TypeWellFormedFromEnv(..) => None,
}
}
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/outlives/explicit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ impl<'tcx> ExplicitPredicatesMap<'tcx> {
| ty::PredicateKind::Coerce(..)
| ty::PredicateKind::ConstEvaluatable(..)
| ty::PredicateKind::ConstEquate(..)
| ty::PredicateKind::Ambiguous
| ty::PredicateKind::TypeWellFormedFromEnv(..) => (),
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// code is looking for a self type of an unresolved
// inference variable.
| ty::PredicateKind::ClosureKind(..)
| ty::PredicateKind::Ambiguous
| ty::PredicateKind::TypeWellFormedFromEnv(..) => None,
},
)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
| ty::PredicateKind::TypeOutlives(..)
| ty::PredicateKind::ConstEvaluatable(..)
| ty::PredicateKind::ConstEquate(..)
| ty::PredicateKind::Ambiguous
| ty::PredicateKind::TypeWellFormedFromEnv(..) => None,
}
});
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ impl<'tcx> InferCtxt<'tcx> {
.normalize_fn_sig_for_diagnostic
.as_ref()
.map(|f| f.clone()),
intercrate: self.intercrate,
}
}
}
Expand Down
13 changes: 2 additions & 11 deletions compiler/rustc_infer/src/infer/canonical/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ use rustc_index::vec::Idx;
use rustc_index::vec::IndexVec;
use rustc_middle::arena::ArenaAllocatable;
use rustc_middle::mir::ConstraintCategory;
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::relate::TypeRelation;
use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
use rustc_middle::ty::{self, BoundVar, Const, ToPredicate, Ty, TyCtxt};
use rustc_middle::ty::{self, BoundVar, ToPredicate, Ty, TyCtxt};
use rustc_span::Span;
use std::fmt::Debug;
use std::iter;
Expand Down Expand Up @@ -729,10 +728,6 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for QueryTypeRelatingDelegate<'_, 'tcx> {
});
}

fn const_equate(&mut self, _a: Const<'tcx>, _b: Const<'tcx>) {
span_bug!(self.cause.span(), "generic_const_exprs: unreachable `const_equate`");
}

fn normalization() -> NormalizationStrategy {
NormalizationStrategy::Eager
}
Expand All @@ -741,11 +736,7 @@ impl<'tcx> TypeRelatingDelegate<'tcx> for QueryTypeRelatingDelegate<'_, 'tcx> {
true
}

fn register_opaque_type_obligations(
&mut self,
obligations: PredicateObligations<'tcx>,
) -> Result<(), TypeError<'tcx>> {
fn register_obligations(&mut self, obligations: PredicateObligations<'tcx>) {
self.obligations.extend(obligations);
Ok(())
}
}
31 changes: 31 additions & 0 deletions compiler/rustc_infer/src/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,15 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> {
ty::Binder::dummy(predicate),
));
}

pub fn mark_ambiguous(&mut self) {
self.obligations.push(Obligation::new(
self.tcx(),
self.trace.cause.clone(),
self.param_env,
ty::Binder::dummy(ty::PredicateKind::Ambiguous),
));
}
}

struct Generalizer<'cx, 'tcx> {
Expand Down Expand Up @@ -521,6 +530,11 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.infcx.tcx
}

fn intercrate(&self) -> bool {
self.infcx.intercrate
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.param_env
}
Expand All @@ -533,6 +547,10 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
true
}

fn mark_ambiguous(&mut self) {
span_bug!(self.cause.span, "opaque types are handled in `tys`");
}
lcnr marked this conversation as resolved.
Show resolved Hide resolved

fn binders<T>(
&mut self,
a: ty::Binder<'tcx, T>,
Expand Down Expand Up @@ -657,6 +675,10 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
// relatable.
Ok(t)
}
ty::Opaque(def_id, substs) => {
let s = self.relate(substs, substs)?;
Ok(if s == substs { t } else { self.infcx.tcx.mk_opaque(def_id, s) })
}
_ => relate::super_relate_tys(self, t, t),
}?;

Expand Down Expand Up @@ -799,6 +821,11 @@ impl<'tcx> TypeRelation<'tcx> for ConstInferUnifier<'_, 'tcx> {
self.infcx.tcx
}

fn intercrate(&self) -> bool {
assert!(!self.infcx.intercrate);
false
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.param_env
}
Expand All @@ -811,6 +838,10 @@ impl<'tcx> TypeRelation<'tcx> for ConstInferUnifier<'_, 'tcx> {
true
}

fn mark_ambiguous(&mut self) {
bug!()
Copy link
Contributor

Choose a reason for hiding this comment

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

that should be reachable when using generic const exprs when relating a const inference var (from a const param) with an unevaluated constant during coherence

}

fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
_variance: ty::Variance,
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_infer/src/infer/equate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
self.fields.tcx()
}

fn intercrate(&self) -> bool {
self.fields.infcx.intercrate
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
self.fields.param_env
}
Expand All @@ -40,6 +44,10 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
self.a_is_expected
}

fn mark_ambiguous(&mut self) {
self.fields.mark_ambiguous();
}

fn relate_item_substs(
&mut self,
_item_def_id: DefId,
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2937,6 +2937,11 @@ impl<'tcx> TypeRelation<'tcx> for SameTypeModuloInfer<'_, 'tcx> {
self.0.tcx
}

fn intercrate(&self) -> bool {
assert!(!self.0.intercrate);
false
}

fn param_env(&self) -> ty::ParamEnv<'tcx> {
// Unused, only for consts which we treat as always equal
ty::ParamEnv::empty()
Expand All @@ -2950,6 +2955,10 @@ impl<'tcx> TypeRelation<'tcx> for SameTypeModuloInfer<'_, 'tcx> {
true
}

fn mark_ambiguous(&mut self) {
bug!()
}

fn relate_with_variance<T: relate::Relate<'tcx>>(
&mut self,
_variance: ty::Variance,
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_infer/src/infer/glb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ impl<'tcx> TypeRelation<'tcx> for Glb<'_, '_, 'tcx> {
"Glb"
}

fn intercrate(&self) -> bool {
assert!(!self.fields.infcx.intercrate);
false
}

fn tcx(&self) -> TyCtxt<'tcx> {
self.fields.tcx()
}
Expand All @@ -42,6 +47,10 @@ impl<'tcx> TypeRelation<'tcx> for Glb<'_, '_, 'tcx> {
self.a_is_expected
}

fn mark_ambiguous(&mut self) {
bug!("mark_ambiguous used outside of coherence");
}

fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_infer/src/infer/lub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ impl<'tcx> TypeRelation<'tcx> for Lub<'_, '_, 'tcx> {
"Lub"
}

fn intercrate(&self) -> bool {
assert!(!self.fields.infcx.intercrate);
false
}

fn tcx(&self) -> TyCtxt<'tcx> {
self.fields.tcx()
}
Expand All @@ -42,6 +47,10 @@ impl<'tcx> TypeRelation<'tcx> for Lub<'_, '_, 'tcx> {
self.a_is_expected
}

fn mark_ambiguous(&mut self) {
bug!("mark_ambiguous used outside of coherence");
}

fn relate_with_variance<T: Relate<'tcx>>(
&mut self,
variance: ty::Variance,
Expand Down
Loading