Skip to content

Commit 20182a9

Browse files
borsbors
committed
Auto merge of #126139 - compiler-errors:specializes, r=<try>
Only compute `specializes` query if (min)specialization is enabled in the crate of the specializing impl Fixes (after backport) #125197 ### What #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`
2 parents 4dc24ae + 4b188d9 commit 20182a9

File tree

9 files changed

+81
-36
lines changed

9 files changed

+81
-36
lines changed

‎compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

+1
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ provide! { tcx, def_id, other, cdata,
314314
extern_crate => { cdata.extern_crate.map(|c| &*tcx.arena.alloc(c)) }
315315
is_no_builtins => { cdata.root.no_builtins }
316316
symbol_mangling_version => { cdata.root.symbol_mangling_version }
317+
specialization_enabled_in => { cdata.root.specialization_enabled_in }
317318
reachable_non_generics => {
318319
let reachable_non_generics = tcx
319320
.exported_symbols(cdata.cnum)

‎compiler/rustc_metadata/src/rmeta/encoder.rs

+1
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
741741
expn_data,
742742
expn_hashes,
743743
def_path_hash_map,
744+
specialization_enabled_in: tcx.specialization_enabled_in(LOCAL_CRATE),
744745
})
745746
});
746747

‎compiler/rustc_metadata/src/rmeta/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,8 @@ pub(crate) struct CrateRoot {
290290
panic_runtime: bool,
291291
profiler_runtime: bool,
292292
symbol_mangling_version: SymbolManglingVersion,
293+
294+
specialization_enabled_in: bool,
293295
}
294296

295297
/// On-disk representation of `DefId`.

‎compiler/rustc_middle/src/query/mod.rs

+5
Original file line numberDiff line numberDiff line change
@@ -1494,6 +1494,11 @@ rustc_queries! {
14941494
separate_provide_extern
14951495
}
14961496

1497+
query specialization_enabled_in(cnum: CrateNum) -> bool {
1498+
desc { "checking whether the crate enabled `specialization`/`min_specialization`" }
1499+
separate_provide_extern
1500+
}
1501+
14971502
query specializes(_: (DefId, DefId)) -> bool {
14981503
desc { "computing whether impls specialize one another" }
14991504
}

‎compiler/rustc_trait_selection/src/traits/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,7 @@ pub fn provide(providers: &mut Providers) {
623623
*providers = Providers {
624624
specialization_graph_of: specialize::specialization_graph_provider,
625625
specializes: specialize::specializes,
626+
specialization_enabled_in: specialize::specialization_enabled_in,
626627
instantiate_and_check_impossible_predicates,
627628
is_impossible_associated_item,
628629
..*providers

‎compiler/rustc_trait_selection/src/traits/specialize/mod.rs

+17-25
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use rustc_data_structures::fx::FxIndexSet;
2424
use rustc_errors::{codes::*, Diag, EmissionGuarantee};
2525
use rustc_hir::def_id::{DefId, LocalDefId};
2626
use rustc_middle::bug;
27+
use rustc_middle::query::LocalCrate;
2728
use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt, TypeVisitableExt};
2829
use rustc_middle::ty::{GenericArgs, GenericArgsRef};
2930
use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK;
@@ -136,38 +137,29 @@ pub fn translate_args_with_cause<'tcx>(
136137
source_args.rebase_onto(infcx.tcx, source_impl, target_args)
137138
}
138139

140+
pub(super) fn specialization_enabled_in(tcx: TyCtxt<'_>, _: LocalCrate) -> bool {
141+
tcx.features().specialization || tcx.features().min_specialization
142+
}
143+
139144
/// Is `impl1` a specialization of `impl2`?
140145
///
141146
/// Specialization is determined by the sets of types to which the impls apply;
142147
/// `impl1` specializes `impl2` if it applies to a subset of the types `impl2` applies
143148
/// to.
144149
#[instrument(skip(tcx), level = "debug")]
145150
pub(super) fn specializes(tcx: TyCtxt<'_>, (impl1_def_id, impl2_def_id): (DefId, DefId)) -> bool {
146-
// The feature gate should prevent introducing new specializations, but not
147-
// taking advantage of upstream ones.
148-
// If specialization is enabled for this crate then no extra checks are needed.
149-
// If it's not, and either of the `impl`s is local to this crate, then this definitely
150-
// isn't specializing - unless specialization is enabled for the `impl` span,
151-
// e.g. if it comes from an `allow_internal_unstable` macro
152-
let features = tcx.features();
153-
let specialization_enabled = features.specialization || features.min_specialization;
154-
if !specialization_enabled {
155-
if impl1_def_id.is_local() {
156-
let span = tcx.def_span(impl1_def_id);
157-
if !span.allows_unstable(sym::specialization)
158-
&& !span.allows_unstable(sym::min_specialization)
159-
{
160-
return false;
161-
}
162-
}
163-
164-
if impl2_def_id.is_local() {
165-
let span = tcx.def_span(impl2_def_id);
166-
if !span.allows_unstable(sym::specialization)
167-
&& !span.allows_unstable(sym::min_specialization)
168-
{
169-
return false;
170-
}
151+
// We check that the specializing impl comes from a crate that has specialization enabled,
152+
// or if the specializing impl is marked with `allow_internal_unstable`.
153+
//
154+
// We don't really care if the specialized impl (the parent) is in a crate that has
155+
// specialization enabled, since it's not being specialized, and it's already been checked
156+
// for coherence.
157+
if !tcx.specialization_enabled_in(impl1_def_id.krate) {
158+
let span = tcx.def_span(impl1_def_id);
159+
if !span.allows_unstable(sym::specialization)
160+
&& !span.allows_unstable(sym::min_specialization)
161+
{
162+
return false;
171163
}
172164
}
173165

‎tests/ui/coherence/coherence-impls-copy.stderr

+11-11
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,3 @@
1-
error[E0117]: only traits defined in the current crate can be implemented for primitive types
2-
--> $DIR/coherence-impls-copy.rs:5:1
3-
|
4-
LL | impl Copy for i32 {}
5-
| ^^^^^^^^^^^^^^---
6-
| | |
7-
| | `i32` is not defined in the current crate
8-
| impl doesn't use only types from inside the current crate
9-
|
10-
= note: define and implement a trait or new type instead
11-
121
error[E0119]: conflicting implementations of trait `Copy` for type `&NotSync`
132
--> $DIR/coherence-impls-copy.rs:28:1
143
|
@@ -30,6 +19,17 @@ LL | impl Copy for &'static [NotSync] {}
3019
|
3120
= note: define and implement a trait or new type instead
3221

22+
error[E0117]: only traits defined in the current crate can be implemented for primitive types
23+
--> $DIR/coherence-impls-copy.rs:5:1
24+
|
25+
LL | impl Copy for i32 {}
26+
| ^^^^^^^^^^^^^^---
27+
| | |
28+
| | `i32` is not defined in the current crate
29+
| impl doesn't use only types from inside the current crate
30+
|
31+
= note: define and implement a trait or new type instead
32+
3333
error[E0206]: the trait `Copy` cannot be implemented for this type
3434
--> $DIR/coherence-impls-copy.rs:21:15
3535
|

‎tests/ui/specialization/anyid-repro-125197.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//@ aux-build: anyid-repro-125197.rs
2+
//@ check-pass
3+
4+
// Makes sure that we don't check `specializes(impl1, impl2)` for a pair of impls that don't
5+
// actually participate in specialization. Since <https://github.com/rust-lang/rust/pull/122791>,
6+
// we don't treat inductive cycles as errors -- so we may need to winnow more pairs of impls, and
7+
// we try to winnow impls in favor of other impls. However, if we're *inside* the `specializes`
8+
// query, then may have a query cycle if we call `specializes` again!
9+
10+
extern crate anyid_repro_125197;
11+
use anyid_repro_125197::AnyId;
12+
13+
fn main() {
14+
let x = "hello, world";
15+
let y: AnyId = x.into();
16+
let _ = y == x;
17+
}

‎tests/ui/specialization/auxiliary/anyid-repro-125197.rs

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
use std::fmt::Display;
2+
use std::sync::Arc;
3+
4+
pub struct AnyId(());
5+
6+
impl PartialEq<Self> for AnyId {
7+
fn eq(&self, _: &Self) -> bool {
8+
todo!()
9+
}
10+
}
11+
12+
impl<T: Identifier> PartialEq<T> for AnyId {
13+
fn eq(&self, _: &T) -> bool {
14+
todo!()
15+
}
16+
}
17+
18+
impl<T: Identifier> From<T> for AnyId {
19+
fn from(_: T) -> Self {
20+
todo!()
21+
}
22+
}
23+
24+
pub trait Identifier: Display + 'static {}
25+
26+
impl<T> Identifier for T where T: PartialEq + Display + 'static {}

0 commit comments

Comments
 (0)