From 8fc655492b93a56dd11afdd726fd61c785158f1b Mon Sep 17 00:00:00 2001 From: jdonszelmann Date: Tue, 13 Aug 2024 11:50:31 +0200 Subject: [PATCH] merge piece on overlap checks with docs about coherence (based on review comments) --- src/SUMMARY.md | 2 +- src/{traits/overlap.md => coherence.md} | 49 +++++++++++++++++++------ src/solve/invariants.md | 2 +- src/traits/specialization.md | 2 +- 4 files changed, 40 insertions(+), 15 deletions(-) rename src/{traits/overlap.md => coherence.md} (57%) diff --git a/src/SUMMARY.md b/src/SUMMARY.md index ec67f3677..ca0317bb8 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -137,7 +137,6 @@ - [Caching subtleties](./traits/caching.md) - [Implied bounds](./traits/implied-bounds.md) - [Specialization](./traits/specialization.md) - - [Overlap checks](./traits/overlap.md) - [Chalk-based trait solving](./traits/chalk.md) - [Lowering to logic](./traits/lowering-to-logic.md) - [Goals and clauses](./traits/goals-and-clauses.md) @@ -157,6 +156,7 @@ - [Type checking](./type-checking.md) - [Method Lookup](./method-lookup.md) - [Variance](./variance.md) + - [Coherence Checking](./coherence.md) - [Opaque Types](./opaque-types-type-alias-impl-trait.md) - [Inference details](./opaque-types-impl-trait-inference.md) - [Return Position Impl Trait In Trait](./return-position-impl-trait-in-trait.md) diff --git a/src/traits/overlap.md b/src/coherence.md similarity index 57% rename from src/traits/overlap.md rename to src/coherence.md index ebd7df8ad..a34e1e8dd 100644 --- a/src/traits/overlap.md +++ b/src/coherence.md @@ -1,17 +1,40 @@ -# Overlap checks -As part of checking items (specifically: structs, enums, traits, unions), -the compiler checks whether impl blocks overlap, for example because they define the same functions. -This is an example an overlap check. -The same overlap check is done when constructing a [specialization graph](./specialization.md). -Here, trait implementations could overlap, for example because of a conflicting blanket implementation overlapping with some specific implementation. +# Coherence -The overlap check always compares two impls. -In the case of inherent impl blocks, this means that at least for small n, -rustc quite literally compares each impl to each other impl block in an `n^2` loop -(see `fn check_item` in coherence/inherent_impls_overlap.rs). +> NOTE: this is based on [notes by @lcnr](https://github.com/rust-lang/rust/pull/121848) + +Coherence checking is what detects both of trait impls and inherent impls overlapping with others. +(reminder: [inherent impls](https://doc.rust-lang.org/reference/items/implementations.html#inherent-implementations) are impls of concrete types like `impl MyStruct {}`) + +Overlapping trait impls always produce an error, +while overlapping inherent impls result in an error only if they have methods with the same name. + +Checking for overlaps is split in two parts. First there's the [overlap check(s)](#overlap-checks), +which finds overlaps between traits and inherent implementations that the compiler currently knows about. + +However, Coherence also results in an error if any other impls **could** exist, +even if they are currently unknown. +This affects impls which may get added to upstream crates in a backwards compatible way, +and impls from downstream crates. +This is called the Orphan check. + +## Overlap checks + +Overlap checks are performed for both inherent impls, and for trait impls. +This uses the same overlap checking code, really done as two separate analyses. +Overlap checks always consider pairs of implementations, comparing them to each other. + +Overlap checking for inherent impl blocks is done through `fn check_item` in coherence/inherent_impls_overlap.rs), +where you can very clearly see that (at least for small `n`), the check really performs `n^2` +comparisons between impls. + +In the case of traits, this check is currently done as part of building the [specialization graph](./specialization.md), +to handle specializing impls overlapping with their parent, but this may change in the future. + +In both cases, all pairs of impls are checked for overlap. Overlapping is sometimes partially allowed: + 1. for maker traits 2. under [specialization](./specialization.md) @@ -23,7 +46,7 @@ Both try to apply negative reasoning to prove that an overlap is definitely impo [`OverlapMode`]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_middle/traits/specialization_graph/enum.OverlapMode.html -## The explicit negative impl check +### The explicit negative impl check This check is done in [`impl_intersection_has_negative_obligation`]. @@ -50,7 +73,7 @@ This is not currently stable. [`impl_intersection_has_negative_obligation`]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_trait_selection/traits/coherence/fn.impl_intersection_has_impossible_obligation.htmlhttps://doc.rust-lang.org/beta/nightly-rustc/rustc_trait_selection/traits/coherence/fn.impl_intersection_has_negative_obligation.html -## The implicit negative impl check +### The implicit negative impl check This check is done in [`impl_intersection_has_impossible_obligation`], and does not rely on negative trait implementations and is stable. @@ -70,3 +93,5 @@ Importantly, this works even if there isn't a `impl !Error for MyLocalType`. [`impl_intersection_has_impossible_obligation`]: https://doc.rust-lang.org/beta/nightly-rustc/rustc_trait_selection/traits/coherence/fn.impl_intersection_has_impossible_obligation.html + + diff --git a/src/solve/invariants.md b/src/solve/invariants.md index 3157d163e..3d29b26ac 100644 --- a/src/solve/invariants.md +++ b/src/solve/invariants.md @@ -113,7 +113,7 @@ in the trait solver #### The type system is complete during the implicit negative overlap check in coherence ✅ -For more on overlap checking: [./overlap.md] +For more on overlap checking: [../coherence.md] During the implicit negative overlap check in coherence we must never return *error* for goals which can be proven. This would allow for overlapping impls with potentially different diff --git a/src/traits/specialization.md b/src/traits/specialization.md index fb6dded46..99b3cda27 100644 --- a/src/traits/specialization.md +++ b/src/traits/specialization.md @@ -5,7 +5,7 @@ Defined in the `specialize` module. The basic strategy is to build up a *specialization graph* during -coherence checking (coherence checking looks for [overlapping impls](./overlap.md)). +coherence checking (coherence checking looks for [overlapping impls](../coherence.md)). Insertion into the graph locates the right place to put an impl in the specialization hierarchy; if there is no right place (due to partial overlap but no containment), you get an overlap