Skip to content

Commit e811e29

Browse files
authoredJul 22, 2020
Rollup merge of #74454 - lcnr:negative-impls, r=nikomatsakis
small coherence cleanup r? @eddyb
2 parents 216ed3c + cfcbca6 commit e811e29

File tree

1 file changed

+47
-52
lines changed

1 file changed

+47
-52
lines changed
 

‎src/librustc_trait_selection/traits/coherence.rs

+47-52
Original file line numberDiff line numberDiff line change
@@ -289,11 +289,11 @@ pub fn orphan_check(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Result<(), OrphanChe
289289
/// - but (knowing that `Vec<T>` is non-fundamental, and assuming it's
290290
/// not local), `Vec<LocalType>` is bad, because `Vec<->` is between
291291
/// the local type and the type parameter.
292-
/// 3. Every type parameter before the local key parameter is fully known in C.
293-
/// - e.g., `impl<T> T: Trait<LocalType>` is bad, because `T` might be
294-
/// an unknown type.
295-
/// - but `impl<T> LocalType: Trait<T>` is OK, because `LocalType`
296-
/// occurs before `T`.
292+
/// 3. Before this local type, no generic type parameter of the impl must
293+
/// be reachable through fundamental types.
294+
/// - e.g. `impl<T> Trait<LocalType> for Vec<T>` is fine, as `Vec` is not fundamental.
295+
/// - while `impl<T> Trait<LocalType for Box<T>` results in an error, as `T` is
296+
/// reachable through the fundamental type `Box`.
297297
/// 4. Every type in the local key parameter not known in C, going
298298
/// through the parameter's type tree, must appear only as a subtree of
299299
/// a type local to C, with only fundamental types between the type
@@ -387,9 +387,9 @@ fn orphan_check_trait_ref<'tcx>(
387387
ty: Ty<'tcx>,
388388
in_crate: InCrate,
389389
) -> Vec<Ty<'tcx>> {
390-
// FIXME(eddyb) figure out if this is redundant with `ty_is_non_local`,
391-
// or maybe if this should be calling `ty_is_non_local_constructor`.
392-
if ty_is_non_local(tcx, ty, in_crate).is_some() {
390+
// FIXME: this is currently somewhat overly complicated,
391+
// but fixing this requires a more complicated refactor.
392+
if !contained_non_local_types(tcx, ty, in_crate).is_empty() {
393393
if let Some(inner_tys) = fundamental_ty_inner_tys(tcx, ty) {
394394
return inner_tys
395395
.flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate))
@@ -408,8 +408,8 @@ fn orphan_check_trait_ref<'tcx>(
408408
.enumerate()
409409
{
410410
debug!("orphan_check_trait_ref: check ty `{:?}`", input_ty);
411-
let non_local_tys = ty_is_non_local(tcx, input_ty, in_crate);
412-
if non_local_tys.is_none() {
411+
let non_local_tys = contained_non_local_types(tcx, input_ty, in_crate);
412+
if non_local_tys.is_empty() {
413413
debug!("orphan_check_trait_ref: ty_is_local `{:?}`", input_ty);
414414
return Ok(());
415415
} else if let ty::Param(_) = input_ty.kind {
@@ -418,37 +418,45 @@ fn orphan_check_trait_ref<'tcx>(
418418
.substs
419419
.types()
420420
.flat_map(|ty| uncover_fundamental_ty(tcx, ty, in_crate))
421-
.find(|ty| ty_is_non_local_constructor(ty, in_crate).is_none());
421+
.find(|ty| ty_is_local_constructor(ty, in_crate));
422422

423423
debug!("orphan_check_trait_ref: uncovered ty local_type: `{:?}`", local_type);
424424

425425
return Err(OrphanCheckErr::UncoveredTy(input_ty, local_type));
426426
}
427-
if let Some(non_local_tys) = non_local_tys {
428-
for input_ty in non_local_tys {
429-
non_local_spans.push((input_ty, i == 0));
430-
}
427+
428+
for input_ty in non_local_tys {
429+
non_local_spans.push((input_ty, i == 0));
431430
}
432431
}
433432
// If we exit above loop, never found a local type.
434433
debug!("orphan_check_trait_ref: no local type");
435434
Err(OrphanCheckErr::NonLocalInputType(non_local_spans))
436435
}
437436

438-
fn ty_is_non_local(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, in_crate: InCrate) -> Option<Vec<Ty<'tcx>>> {
439-
match ty_is_non_local_constructor(ty, in_crate) {
440-
Some(ty) => {
441-
if let Some(inner_tys) = fundamental_ty_inner_tys(tcx, ty) {
442-
let tys: Vec<_> = inner_tys
443-
.filter_map(|ty| ty_is_non_local(tcx, ty, in_crate))
444-
.flatten()
445-
.collect();
446-
if tys.is_empty() { None } else { Some(tys) }
447-
} else {
448-
Some(vec![ty])
437+
/// Returns a list of relevant non-local types for `ty`.
438+
///
439+
/// This is just `ty` itself unless `ty` is `#[fundamental]`,
440+
/// in which case we recursively look into this type.
441+
///
442+
/// If `ty` is local itself, this method returns an empty `Vec`.
443+
///
444+
/// # Examples
445+
///
446+
/// - `u32` is not local, so this returns `[u32]`.
447+
/// - for `Foo<u32>`, where `Foo` is a local type, this returns `[]`.
448+
/// - `&mut u32` returns `[u32]`, as `&mut` is a fundamental type, similar to `Box`.
449+
/// - `Box<Foo<u32>>` returns `[]`, as `Box` is a fundamental type and `Foo` is local.
450+
fn contained_non_local_types(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, in_crate: InCrate) -> Vec<Ty<'tcx>> {
451+
if ty_is_local_constructor(ty, in_crate) {
452+
Vec::new()
453+
} else {
454+
match fundamental_ty_inner_tys(tcx, ty) {
455+
Some(inner_tys) => {
456+
inner_tys.flat_map(|ty| contained_non_local_types(tcx, ty, in_crate)).collect()
449457
}
458+
None => vec![ty],
450459
}
451-
None => None,
452460
}
453461
}
454462

@@ -493,9 +501,8 @@ fn def_id_is_local(def_id: DefId, in_crate: InCrate) -> bool {
493501
}
494502
}
495503

496-
// FIXME(eddyb) this can just return `bool` as it always returns `Some(ty)` or `None`.
497-
fn ty_is_non_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> Option<Ty<'_>> {
498-
debug!("ty_is_non_local_constructor({:?})", ty);
504+
fn ty_is_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> bool {
505+
debug!("ty_is_local_constructor({:?})", ty);
499506

500507
match ty.kind {
501508
ty::Bool
@@ -513,29 +520,17 @@ fn ty_is_non_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> Option<Ty<'_>>
513520
| ty::Never
514521
| ty::Tuple(..)
515522
| ty::Param(..)
516-
| ty::Projection(..) => Some(ty),
523+
| ty::Projection(..) => false,
517524

518525
ty::Placeholder(..) | ty::Bound(..) | ty::Infer(..) => match in_crate {
519-
InCrate::Local => Some(ty),
526+
InCrate::Local => false,
520527
// The inference variable might be unified with a local
521528
// type in that remote crate.
522-
InCrate::Remote => None,
529+
InCrate::Remote => true,
523530
},
524531

525-
ty::Adt(def, _) => {
526-
if def_id_is_local(def.did, in_crate) {
527-
None
528-
} else {
529-
Some(ty)
530-
}
531-
}
532-
ty::Foreign(did) => {
533-
if def_id_is_local(did, in_crate) {
534-
None
535-
} else {
536-
Some(ty)
537-
}
538-
}
532+
ty::Adt(def, _) => def_id_is_local(def.did, in_crate),
533+
ty::Foreign(did) => def_id_is_local(did, in_crate),
539534
ty::Opaque(..) => {
540535
// This merits some explanation.
541536
// Normally, opaque types are not involed when performing
@@ -553,7 +548,7 @@ fn ty_is_non_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> Option<Ty<'_>>
553548
// the underlying type *within the same crate*. When an
554549
// opaque type is used from outside the module
555550
// where it is declared, it should be impossible to observe
556-
// anyything about it other than the traits that it implements.
551+
// anything about it other than the traits that it implements.
557552
//
558553
// The alternative would be to look at the underlying type
559554
// to determine whether or not the opaque type itself should
@@ -562,18 +557,18 @@ fn ty_is_non_local_constructor(ty: Ty<'_>, in_crate: InCrate) -> Option<Ty<'_>>
562557
// to a remote type. This would violate the rule that opaque
563558
// types should be completely opaque apart from the traits
564559
// that they implement, so we don't use this behavior.
565-
Some(ty)
560+
false
566561
}
567562

568563
ty::Dynamic(ref tt, ..) => {
569564
if let Some(principal) = tt.principal() {
570-
if def_id_is_local(principal.def_id(), in_crate) { None } else { Some(ty) }
565+
def_id_is_local(principal.def_id(), in_crate)
571566
} else {
572-
Some(ty)
567+
false
573568
}
574569
}
575570

576-
ty::Error(_) => None,
571+
ty::Error(_) => true,
577572

578573
ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => {
579574
bug!("ty_is_local invoked on unexpected type: {:?}", ty)

0 commit comments

Comments
 (0)
Please sign in to comment.