Auto merge of #126139 - compiler-errors:specializes, r=lcnr
Only compute `specializes` query if (min)specialization is enabled in the crate of the specializing impl Fixes (after backport) https://github.com/rust-lang/rust/issues/125197 ### What https://github.com/rust-lang/rust/pull/122791 makes it so that inductive cycles are no longer hard errors. That means that when we are testing, for example, whether these impls overlap: ```rust impl PartialEq<Self> for AnyId { fn eq(&self, _: &Self) -> bool { todo!() } } impl<T: Identifier> PartialEq<T> for AnyId { fn eq(&self, _: &T) -> bool { todo!() } } ``` ...given... ```rust pub trait Identifier: Display + 'static {} impl<T> Identifier for T where T: PartialEq + Display + 'static {} ``` Then we try to see if the second impl holds given `T = AnyId`. That requires `AnyId: Identifier`, which requires that `AnyId: PartialEq`, which is satisfied by these two impl candidates... The `PartialEq<T>` impl is a cycle, and we used to winnow it when we used to treat inductive cycles as errors. However, now that we don't winnow it, this means that we *now* try calling `candidate_should_be_dropped_in_favor_of`, which tries to check whether one of the impls specializes the other: the `specializes` query. In that query, we currently bail early if the impl is local. However, in a foreign crate, we try to compute if the two impls specialize each other by doing trait solving. This may itself lead to the same situation where we call `specializes`, which will lead to a query cycle. ### How does this fix the problem We now record whether specialization is enabled in foreign crates, and extend this early-return behavior to foreign impls too. This means that we can only encounter these cycles if we truly have a specializing impl from a crate with specialization enabled. ----- r? `@oli-obk` or `@lcnr`
This commit is contained in:
commit
336e6ab3b3
9 changed files with 81 additions and 36 deletions
|
@ -314,6 +314,7 @@ provide! { tcx, def_id, other, cdata,
|
||||||
extern_crate => { cdata.extern_crate.map(|c| &*tcx.arena.alloc(c)) }
|
extern_crate => { cdata.extern_crate.map(|c| &*tcx.arena.alloc(c)) }
|
||||||
is_no_builtins => { cdata.root.no_builtins }
|
is_no_builtins => { cdata.root.no_builtins }
|
||||||
symbol_mangling_version => { cdata.root.symbol_mangling_version }
|
symbol_mangling_version => { cdata.root.symbol_mangling_version }
|
||||||
|
specialization_enabled_in => { cdata.root.specialization_enabled_in }
|
||||||
reachable_non_generics => {
|
reachable_non_generics => {
|
||||||
let reachable_non_generics = tcx
|
let reachable_non_generics = tcx
|
||||||
.exported_symbols(cdata.cnum)
|
.exported_symbols(cdata.cnum)
|
||||||
|
|
|
@ -741,6 +741,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
|
||||||
expn_data,
|
expn_data,
|
||||||
expn_hashes,
|
expn_hashes,
|
||||||
def_path_hash_map,
|
def_path_hash_map,
|
||||||
|
specialization_enabled_in: tcx.specialization_enabled_in(LOCAL_CRATE),
|
||||||
})
|
})
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
@ -290,6 +290,8 @@ pub(crate) struct CrateRoot {
|
||||||
panic_runtime: bool,
|
panic_runtime: bool,
|
||||||
profiler_runtime: bool,
|
profiler_runtime: bool,
|
||||||
symbol_mangling_version: SymbolManglingVersion,
|
symbol_mangling_version: SymbolManglingVersion,
|
||||||
|
|
||||||
|
specialization_enabled_in: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// On-disk representation of `DefId`.
|
/// On-disk representation of `DefId`.
|
||||||
|
|
|
@ -1494,6 +1494,11 @@ rustc_queries! {
|
||||||
separate_provide_extern
|
separate_provide_extern
|
||||||
}
|
}
|
||||||
|
|
||||||
|
query specialization_enabled_in(cnum: CrateNum) -> bool {
|
||||||
|
desc { "checking whether the crate enabled `specialization`/`min_specialization`" }
|
||||||
|
separate_provide_extern
|
||||||
|
}
|
||||||
|
|
||||||
query specializes(_: (DefId, DefId)) -> bool {
|
query specializes(_: (DefId, DefId)) -> bool {
|
||||||
desc { "computing whether impls specialize one another" }
|
desc { "computing whether impls specialize one another" }
|
||||||
}
|
}
|
||||||
|
|
|
@ -623,6 +623,7 @@ pub fn provide(providers: &mut Providers) {
|
||||||
*providers = Providers {
|
*providers = Providers {
|
||||||
specialization_graph_of: specialize::specialization_graph_provider,
|
specialization_graph_of: specialize::specialization_graph_provider,
|
||||||
specializes: specialize::specializes,
|
specializes: specialize::specializes,
|
||||||
|
specialization_enabled_in: specialize::specialization_enabled_in,
|
||||||
instantiate_and_check_impossible_predicates,
|
instantiate_and_check_impossible_predicates,
|
||||||
is_impossible_associated_item,
|
is_impossible_associated_item,
|
||||||
..*providers
|
..*providers
|
||||||
|
|
|
@ -24,6 +24,7 @@ use rustc_data_structures::fx::FxIndexSet;
|
||||||
use rustc_errors::{codes::*, Diag, EmissionGuarantee};
|
use rustc_errors::{codes::*, Diag, EmissionGuarantee};
|
||||||
use rustc_hir::def_id::{DefId, LocalDefId};
|
use rustc_hir::def_id::{DefId, LocalDefId};
|
||||||
use rustc_middle::bug;
|
use rustc_middle::bug;
|
||||||
|
use rustc_middle::query::LocalCrate;
|
||||||
use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt, TypeVisitableExt};
|
use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt, TypeVisitableExt};
|
||||||
use rustc_middle::ty::{GenericArgs, GenericArgsRef};
|
use rustc_middle::ty::{GenericArgs, GenericArgsRef};
|
||||||
use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK;
|
use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK;
|
||||||
|
@ -136,6 +137,10 @@ pub fn translate_args_with_cause<'tcx>(
|
||||||
source_args.rebase_onto(infcx.tcx, source_impl, target_args)
|
source_args.rebase_onto(infcx.tcx, source_impl, target_args)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub(super) fn specialization_enabled_in(tcx: TyCtxt<'_>, _: LocalCrate) -> bool {
|
||||||
|
tcx.features().specialization || tcx.features().min_specialization
|
||||||
|
}
|
||||||
|
|
||||||
/// Is `impl1` a specialization of `impl2`?
|
/// Is `impl1` a specialization of `impl2`?
|
||||||
///
|
///
|
||||||
/// Specialization is determined by the sets of types to which the impls apply;
|
/// Specialization is determined by the sets of types to which the impls apply;
|
||||||
|
@ -143,31 +148,18 @@ pub fn translate_args_with_cause<'tcx>(
|
||||||
/// to.
|
/// to.
|
||||||
#[instrument(skip(tcx), level = "debug")]
|
#[instrument(skip(tcx), level = "debug")]
|
||||||
pub(super) fn specializes(tcx: TyCtxt<'_>, (impl1_def_id, impl2_def_id): (DefId, DefId)) -> bool {
|
pub(super) fn specializes(tcx: TyCtxt<'_>, (impl1_def_id, impl2_def_id): (DefId, DefId)) -> bool {
|
||||||
// The feature gate should prevent introducing new specializations, but not
|
// We check that the specializing impl comes from a crate that has specialization enabled,
|
||||||
// taking advantage of upstream ones.
|
// or if the specializing impl is marked with `allow_internal_unstable`.
|
||||||
// If specialization is enabled for this crate then no extra checks are needed.
|
//
|
||||||
// If it's not, and either of the `impl`s is local to this crate, then this definitely
|
// We don't really care if the specialized impl (the parent) is in a crate that has
|
||||||
// isn't specializing - unless specialization is enabled for the `impl` span,
|
// specialization enabled, since it's not being specialized, and it's already been checked
|
||||||
// e.g. if it comes from an `allow_internal_unstable` macro
|
// for coherence.
|
||||||
let features = tcx.features();
|
if !tcx.specialization_enabled_in(impl1_def_id.krate) {
|
||||||
let specialization_enabled = features.specialization || features.min_specialization;
|
let span = tcx.def_span(impl1_def_id);
|
||||||
if !specialization_enabled {
|
if !span.allows_unstable(sym::specialization)
|
||||||
if impl1_def_id.is_local() {
|
&& !span.allows_unstable(sym::min_specialization)
|
||||||
let span = tcx.def_span(impl1_def_id);
|
{
|
||||||
if !span.allows_unstable(sym::specialization)
|
return false;
|
||||||
&& !span.allows_unstable(sym::min_specialization)
|
|
||||||
{
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if impl2_def_id.is_local() {
|
|
||||||
let span = tcx.def_span(impl2_def_id);
|
|
||||||
if !span.allows_unstable(sym::specialization)
|
|
||||||
&& !span.allows_unstable(sym::min_specialization)
|
|
||||||
{
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1,14 +1,3 @@
|
||||||
error[E0117]: only traits defined in the current crate can be implemented for primitive types
|
|
||||||
--> $DIR/coherence-impls-copy.rs:5:1
|
|
||||||
|
|
|
||||||
LL | impl Copy for i32 {}
|
|
||||||
| ^^^^^^^^^^^^^^---
|
|
||||||
| | |
|
|
||||||
| | `i32` is not defined in the current crate
|
|
||||||
| impl doesn't use only types from inside the current crate
|
|
||||||
|
|
|
||||||
= note: define and implement a trait or new type instead
|
|
||||||
|
|
||||||
error[E0119]: conflicting implementations of trait `Copy` for type `&NotSync`
|
error[E0119]: conflicting implementations of trait `Copy` for type `&NotSync`
|
||||||
--> $DIR/coherence-impls-copy.rs:28:1
|
--> $DIR/coherence-impls-copy.rs:28:1
|
||||||
|
|
|
|
||||||
|
@ -30,6 +19,17 @@ LL | impl Copy for &'static [NotSync] {}
|
||||||
|
|
|
|
||||||
= note: define and implement a trait or new type instead
|
= note: define and implement a trait or new type instead
|
||||||
|
|
||||||
|
error[E0117]: only traits defined in the current crate can be implemented for primitive types
|
||||||
|
--> $DIR/coherence-impls-copy.rs:5:1
|
||||||
|
|
|
||||||
|
LL | impl Copy for i32 {}
|
||||||
|
| ^^^^^^^^^^^^^^---
|
||||||
|
| | |
|
||||||
|
| | `i32` is not defined in the current crate
|
||||||
|
| impl doesn't use only types from inside the current crate
|
||||||
|
|
|
||||||
|
= note: define and implement a trait or new type instead
|
||||||
|
|
||||||
error[E0206]: the trait `Copy` cannot be implemented for this type
|
error[E0206]: the trait `Copy` cannot be implemented for this type
|
||||||
--> $DIR/coherence-impls-copy.rs:21:15
|
--> $DIR/coherence-impls-copy.rs:21:15
|
||||||
|
|
|
|
||||||
|
|
17
tests/ui/specialization/anyid-repro-125197.rs
Normal file
17
tests/ui/specialization/anyid-repro-125197.rs
Normal file
|
@ -0,0 +1,17 @@
|
||||||
|
//@ aux-build: anyid-repro-125197.rs
|
||||||
|
//@ check-pass
|
||||||
|
|
||||||
|
// Makes sure that we don't check `specializes(impl1, impl2)` for a pair of impls that don't
|
||||||
|
// actually participate in specialization. Since <https://github.com/rust-lang/rust/pull/122791>,
|
||||||
|
// we don't treat inductive cycles as errors -- so we may need to winnow more pairs of impls, and
|
||||||
|
// we try to winnow impls in favor of other impls. However, if we're *inside* the `specializes`
|
||||||
|
// query, then may have a query cycle if we call `specializes` again!
|
||||||
|
|
||||||
|
extern crate anyid_repro_125197;
|
||||||
|
use anyid_repro_125197::AnyId;
|
||||||
|
|
||||||
|
fn main() {
|
||||||
|
let x = "hello, world";
|
||||||
|
let y: AnyId = x.into();
|
||||||
|
let _ = y == x;
|
||||||
|
}
|
26
tests/ui/specialization/auxiliary/anyid-repro-125197.rs
Normal file
26
tests/ui/specialization/auxiliary/anyid-repro-125197.rs
Normal file
|
@ -0,0 +1,26 @@
|
||||||
|
use std::fmt::Display;
|
||||||
|
use std::sync::Arc;
|
||||||
|
|
||||||
|
pub struct AnyId(());
|
||||||
|
|
||||||
|
impl PartialEq<Self> for AnyId {
|
||||||
|
fn eq(&self, _: &Self) -> bool {
|
||||||
|
todo!()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<T: Identifier> PartialEq<T> for AnyId {
|
||||||
|
fn eq(&self, _: &T) -> bool {
|
||||||
|
todo!()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<T: Identifier> From<T> for AnyId {
|
||||||
|
fn from(_: T) -> Self {
|
||||||
|
todo!()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
pub trait Identifier: Display + 'static {}
|
||||||
|
|
||||||
|
impl<T> Identifier for T where T: PartialEq + Display + 'static {}
|
Loading…
Add table
Add a link
Reference in a new issue