bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #99666 - compiler-errors:issue-99663, r=lcnr

Browse source 
Restore `Opaque` behavior to coherence check

Fixes #99663.

This broke in 84c3fcd2a0. I'm not exactly certain that adding this behavior back is necessarily correct, but at least the UI test I provided may stimulate some thoughts.

I think delaying a bug here is certainly not correct in the case of opaques -- if we want to change coherence behavior for opaques, then we should at least be emitting a new error.

r? ``@lcnr``
This commit is contained in:
Dylan DPC 2022-07-26 14:26:57 +05:30 • committed by GitHub
commit 99350de0d5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 75 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

31
compiler/rustc_trait_selection/src/traits/coherence.rs
View file

@ -703,13 +703,42 @@ impl<'tcx> TypeVisitor<'tcx> for OrphanChecker<'tcx> {
}
}
ty::Error(_) => ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty)),
ty::Opaque(..) | ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => {
ty::Closure(..) | ty::Generator(..) | ty::GeneratorWitness(..) => {
self.tcx.sess.delay_span_bug(
DUMMY_SP,
format!("ty_is_local invoked on closure or generator: {:?}", ty),
);
ControlFlow::Break(OrphanCheckEarlyExit::LocalTy(ty))
}
ty::Opaque(..) => {
// This merits some explanation.
// Normally, opaque types are not involved when performing
// coherence checking, since it is illegal to directly
// implement a trait on an opaque type. However, we might
// end up looking at an opaque type during coherence checking
// if an opaque type gets used within another type (e.g. as
// the type of a field) when checking for auto trait or `Sized`
// impls. This requires us to decide whether or not an opaque
// type should be considered 'local' or not.
//
// We choose to treat all opaque types as non-local, even
// those that appear within the same crate. This seems
// somewhat surprising at first, but makes sense when
// you consider that opaque types are supposed to hide
// the underlying type *within the same crate*. When an
// opaque type is used from outside the module
// where it is declared, it should be impossible to observe
// anything about it other than the traits that it implements.
//
// The alternative would be to look at the underlying type
// to determine whether or not the opaque type itself should
// be considered local. However, this could make it a breaking change
// to switch the underlying ('defining') type from a local type
// to a remote type. This would violate the rule that opaque
// types should be completely opaque apart from the traits
// that they implement, so we don't use this behavior.
self.found_non_local_ty(ty)
}
};
// A bit of a hack, the `OrphanChecker` is only used to visit a `TraitRef`, so
// the first type we visit is always the self type.