#48538 Specialization: Intersection Impls #49624

giannicic · 2018-04-03T14:32:46Z

This commit implements the Intersection Impls feature described in this blog post.

The intuition is pretty simple: if you have two impls that have a partial intersection, but which don’t strictly subset one another, then you can add a third impl that covers precisely that intersection, and hence which subsets both of them. So now, for any type, there is always a “most specific” impl to choose.

It'a preparatory work for the implementation of the "always applicable impls" feature.

I've some questions/doubts about the implementation:

the check_specialization_validity method on an intersection impl should check if both parents have the default keyword? I think that at the moment it checks only the immediate parent.
should this PR include a compiler message note like the one described here or we could skip it for the moment to go forward on the always applicable impls and then return at it later?

r? @nikomatsakis

TimNN · 2018-04-03T15:17:32Z

Your PR failed on Travis. Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

Resolving deltas: 100% (611330/611330), completed with 4859 local objects.
---
[00:00:46] configure: rust.quiet-tests     := True
---
[00:40:30] ..........................................................................i.........................
[00:40:36] .................i.........................................................F........................
---
[00:41:11] .............................................................................................i......
[00:41:18] .................................................................i..................................
---
[00:41:40] - error[E0119]: conflicting implementations of trait `std::ops::Deref` for type `&_`:
[00:41:40] + error[E0119]: conflicting implementations of trait `std::ops::Deref` for type `std::cell::Ref<'_, _>`:
[00:41:40] 2   --> $DIR/issue-28981.rs:15:1
[00:41:40] 3    |
[00:41:40] 4 LL | impl<Foo> Deref for Foo { } //~ ERROR must be used
[00:41:40]
[00:41:40] 5    | ^^^^^^^^^^^^^^^^^^^^^^^
[00:41:40] 6    |
[00:41:40] 7    = note: conflicting implementation in crate `core`:
[00:41:40] -            - impl<'a, T> std::ops::Deref for &'a T
[00:41:40] +            - impl<'b, T> std::ops::Deref for std::cell::Ref<'b, T>
[00:41:40] 9              where T: ?Sized;
[00:41:40] 10
[00:41:40] 11 error[E0210]: type parameter `Foo` must be used as the type parameter for some local type (e.g. `MyStruct<Foo>`)
[00:41:40]
[00:41:40]
[00:41:40] The actual stderr differed from the expected stderr.
[00:41:40] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/e0119/issue-28981.stderr
[00:41:40] To update references, run this command from build directory:
[00:41:40] /checkout/src/test/ui/update-references.sh '/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui' 'e0119/issue-28981.rs'
[00:41:40]
[00:41:40] error: 1 errors occurred comparing output.
[00:41:40] status: exit code: 101
[00:41:40] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/e0119/issue-28981.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/e0119/issue-28981.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/e0119/issue-28981.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
---
[00:41:40] {"message":"conflicting implementations of trait `std::ops::Deref` for type `std::cell::Ref<'_, _>`:","code":{"code":"E0119","explanation":"\nThere are conflicting trait implementations for the same type.\nExample of erroneous code:\n\n```compile_fail,E0119\ntrait MyTrait {\n    fn get(&self) -> usize;\n}\n\nimpl<T> MyTrait for T {\n    fn get(&self) -> usize { 0 }\n}\n\nstruct Foo {\n    value: usize\n}\n\nimpl MyTrait for Foo { // error: conflicting implementations of trait\n                       //        `MyTrait` for type `Foo`\n    fn get(&self) -> usize { self.value }\n}\n```\n\nWhen looking for the implementation for the trait, the compiler finds\nboth the `impl<T> MyTrait for T` where T is all types and the `impl\nMyTrait for Foo`. Since a trait cannot be implemented multiple times,\nthis is an error. So, when you write:\n\n```\ntrait MyTrait {\n    fn get(&self) -> usize;\n}\n\nimpl<T> MyTrait for T {\n    fn get(&self) -> usize { 0 }\n}\n```\n\nThis makes the trait implemented on all types in the scope. So if you\ntry to implement it on another one after that, the implementations will\nconflict. Example:\n\n```\ntrait MyTrait {\n    fn get(&self) -> usize;\n}\n\nimpl<T> MyTrait for T {\n    fn get(&self) -> usize { 0 }\n}\n\nstruct Foo;\n\nfn main() {\n    let f = Foo;\n\n    f.get(); // the trait is implemented so we can use it\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/e0119/issue-28981.rs","byte_start":502,"byte_end":525,"line_start":15,"line_end":15,"column_start":1,"column_end":24,"is_primary":true,"text":[{"text":"impl<Foo> Deref for Foo { } //~ ERROR must be used","highlight_start":1,"highlight_end":24}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[{"message":"conflicting implementation in crate `core`:\n- impl<'b, T> std::ops::Deref for std::cell::Ref<'b, T>\n  where T: ?Sized;","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0119]: conflicting implementations of trait `std::ops::Deref` for type `std::cell::Ref<'_, _>`:\n  --> /checkout/src/test/ui/e0119/issue-28981.rs:15:1\n   |\nLL | impl<Foo> Deref for Foo { } //~ ERROR must be used\n   | ^^^^^^^^^^^^^^^^^^^^^^^\n   |\n   = note: conflicting implementation in crate `core`:\n           - impl<'b, T> std::ops::Deref for std::cell::Ref<'b, T>\n             where T: ?Sized;\n\n"}
[00:41:40] {"message":"type parameter `Foo` must be used as the type parameter for some local type (e.g. `MyStruct<Foo>`)","code":{"code":"E0210","explanation":"\nThis error indicates a violation of one of Rust's orphan rules for trait\nimplementations. The rule concerns the use of type parameters in an\nimplementation of a foreign trait (a trait defined in another crate), and\nstates that type parameters must be \"covered\" by a local type. To understand\nwhat this means, it is perhaps easiest to consider a few examples.\n\nIf `ForeignTrait` is a trait defined in some external crate `foo`, then the\nfollowing trait `impl` is an error:\n\n```compile_fail,E0210\n# #[cfg(for_demonstration_only)]\nextern crate foo;\n# #[cfg(for_demonstration_only)]\nuse foo::ForeignTrait;\n# use std::panic::UnwindSafe as ForeignTrait;\n\nimpl<T> ForeignTrait for T { } // error\n# fn main() {}\n```\n\nTo work around this, it can be covered with a local type, `MyType`:\n\n```\n# use std::panic::UnwindSafe as ForeignTrait;\nstruct MyType<T>(T);\nimpl<T> ForeignTrait for MyType<T> { } // Ok\n```\n\nPlease note that a type alias is not sufficient.\n\nFor another example of an error, suppose there's another trait defined in `foo`\nnamed `ForeignTrait2` that takes two type parameters. Then this `impl` results\nin the same rule violation:\n\n```ignore (cannot-doctest-multicrate-project)\nstruct MyType2;\nimpl<T> ForeignTrait2<T, MyType<T>> for MyType2 { } // error\n```\n\nThe reason for this is that there are two appearances of type parameter `T` in\nthe `impl` header, both as parameters for `ForeignTrait2`. The first appearance\nis uncovered, and so runs afoul of the orphan rule.\n\nConsider one more example:\n\n```ignore (cannot-doctest-multicrate-project)\nimpl<T> ForeignTrait2<MyType<T>, T> for MyType2 { } // Ok\n```\n\nThis only differs from the previous `impl` in that the parameters `T` and\n`MyType<T>` for `ForeignTrait2` have been swapped. This example does *not*\nviolate the orphan rule; it is permitted.\n\nTo see why that last example was allowed, you need to understand the general\nrule. Unfortunately this rule is a bit tricky to state. Consider an `impl`:\n\n```ignore (only-for-syntax-highlight)\nimpl<P1, ..., Pm> ForeignTrait<T1, ..., Tn> for T0 { ... }\n```\n\nwhere `P1, ..., Pm` are the type parameters of the `impl` and `T0, ..., Tn`\nare types. One of the types `T0, ..., Tn` must be a local type (this is another\norphan rule, see the explanation for E0117). Let `i` be the smallest integer\nsuch that `Ti` is a local type. Then no type parameter can appear in any of the\n`Tj` for `j < i`.\n\nFor information on the design of the orphan rules, see [RFC 1023].\n\n[RFC 1023]: https://github.com/rust-lang/rfcs/blob/master/text/1023-rebalancing-coherence.md\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/e0119/issue-28981.rs","byte_start":502,"byte_end":525,"line_start":15,"line_end":15,"column_start":1,"column_end":24,"is_primary":true,"text":[{"text":"impl<Foo> Deref for Foo { } //~ ERROR must be used","highlight_start":1,"highlight_end":24}],"label":"type parameter `Foo` must be used as the type parameter for some local type","suggested_replacement":null,"expansion":null}],"children":[{"message":"only traits defined in the current crate can be implemented for a type parameter","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0210]: type parameter `Foo` must be used as the type parameter for some local type (e.g. `MyStruct<Foo>`)\n  --> /checkout/src/test/ui/e0119/issue-28981.rs:15:1\n   |\nLL | impl<Foo> Deref for Foo { } //~ ERROR must be used\n   | ^^^^^^^^^^^^^^^^^^^^^^^ type parameter `Foo` must be used as the type parameter for some local type\n   |\n   = note: only traits defined in the current crate can be implemented for a type parameter\n\n"}
[00:41:40] {"message":"aborting due to 2 previous errors","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to 2 previous errors\n\n"}
[00:41:40] {"message":"Some errors occurred: E0119, E0210.","code":null,"level":"","spans":[],"children":[],"rendered":"Some errors occurred: E0119, E0210.\n"}
[00:41:40] {"message":"For more information about an error, try `rustc --explain E0119`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about an error, try `rustc --explain E0119`.\n"}
---
[00:41:40] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zmiri -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zmiri -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:41:40] expected success, got: exit code: 101
[00:41:40]
[00:41:40]
[00:41:40] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:41:40] Build completed unsuccessfully in 0:01:55
[00:41:40] make: *** [check] Error 1
[00:41:40] Makefile:58: recipe for target 'check' failed
---
$ cat obj/tmp/sccache.log
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:03fbf6a0:start=1522768647643915371,finish=1522768647651434788,duration=7519417
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:02de74ef
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:02de74ef:start=1522768647658523738,finish=1522768647665064969,duration=6541231
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:13ba59ab
$ dmesg | grep -i kill
[   10.885261] init: failsafe main process (1094) killed by TERM signal

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN.

nikomatsakis · 2018-04-04T18:37:16Z

src/librustc/traits/specialize/mod.rs

@@ -42,6 +42,7 @@ pub struct OverlapError {
    pub trait_desc: String,
    pub self_desc: Option<String>,
    pub intercrate_ambiguity_causes: Vec<IntercrateAmbiguityCause>,
+    pub used_to_be_allowed: bool,


what is this about?

used_to_be_allowed is the name of a variable currently used in librustc/traits/specialize/mod.rs.
This variable is setted based on the result of the insertion and is linked to the IntercrateMode used to check if two impls overalp.
Since, with my changes, any OverlapErrors is no more emitted at insertion time I keep track of this variable when creating the OverlapError struct and i use it later when checking if two impls overlap.

nikomatsakis · 2018-04-06T19:15:02Z

src/librustc/traits/specialize/specialization_graph.rs

@@ -291,8 +335,52 @@ impl<'a, 'gcx, 'tcx> Graph {

    /// The parent of a given impl, which is the def id of the trait when the
    /// impl is a "specialization root".
-    pub fn parent(&self, child: DefId) -> DefId {
-        *self.parent.get(&child).unwrap()
+    pub fn parent(&self, child: DefId) -> Vec<DefId> {


Nit: can we rename this to parents?

Also, the comment should be altered. Something like:

Returns the def-id of the parent impl(s) for a given impl -- these are the impls which this impl specializes, meaning that it matches a subset of the types that they match.

Actually, better to phrase more precisely, I think:

Returns the def-id of the parent impl(s) for a given impl. An impl A has a parent impl B if A matches a strict subset of the types that B matches.

nikomatsakis · 2018-04-06T19:17:35Z

src/librustc/traits/specialize/specialization_graph.rs

+        self.parent.get(&child).unwrap().clone()
+    }
+
+    pub fn build_graph(&self) -> (DataStructuresGraph<DefId, String>, DefIdMap<NodeIndex>) {


Comment would be great. What are these two things that are being returned, anyway?

For example, worth documenting that in the graph, we have edges from children to parents

Since the specialization graph doesn't have the computations (Eg. postdominators) needed to check overlaps between impls this method build a "DataStructureGraph" out of a specialization Graph. The data returned are the "DataStructureGraph" and a DefIdMap that gives the NodeIndexes of all the impls in the Graph.
Here I originally thought to replace the specizlization Graph directly with a DataStructureGraph but I wasn't sure about that. Do you think i should replace it considering also what you wrote later?

it feels like it'd be nice to extract out this graph into something we can unit-test, that is more independent of the compiler -- eg., living in rustc_data_structures

it might be nice to replace it, yes, though maybe that should be a separate PR

i.e., it'd be nice to do that refactoring first without changing semantics

nikomatsakis · 2018-04-06T19:22:53Z

src/librustc/traits/specialize/specialization_graph.rs

+                         sg_graph: &DataStructuresGraph<DefId, String>,
+                         nodes_idx: &DefIdMap<NodeIndex>,
+                         impl1: DefId,
+                         impl2: DefId) -> bool {


Nit: can we format this method the rustfmt way?

I've run the rustfmt tool on the entire file. It seems that there were other methods not formatted.

nikomatsakis · 2018-04-06T19:48:44Z

src/librustc/traits/specialize/specialization_graph.rs

+            let idx = sg_graph.add_node(node);
+            nodes_idx.insert(node, idx);
+            idx
+        }


it feels like it'd be nice to extract out this graph into something we can unit-test, that is more independent of the compiler -- eg., living in rustc_data_structures

nikomatsakis · 2018-04-06T20:04:25Z

src/librustc/traits/specialize/specialization_graph.rs

+        let impl2_idx = *nodes_idx.get(&impl2).unwrap();
+
+        let impl1_incoming_nodes = sg_graph.nodes_in_postorder(INCOMING, impl1_idx);
+        let impl2_incoming_nodes = sg_graph.nodes_in_postorder(INCOMING, impl2_idx);


Man, I must be missing something. I don't get why this works. It appears from the code above that the edges go from child to parent, and we are therefore walking from the children to the parents here...maybe I don't understand something? All of this tells me that we need a comment here!

But also, I think that we have to be pretty careful about this check. It's not enough for there to be an impl that specializes both of them: that impl has to cover the full intersection, right?

For example, consider this:

trait A { } trait B { } trait C { } trait MyTrait { } impl<T: A> MyTrait for T { } impl<T: B> MyTrait for T { } // This would be OK: // impl<T: A + B> MyTrait for T { } // But what about this: impl<T: A + B + C> MyTrait for T { }

Does your code accept that?

I'll add your example as a test and let you know.

@nikomatsakis My code accept that since A+B+C specializes both A and B but it shouldn't accept it, Right? Because it extends beyond the intersection of A and B. So an additional check is required in order to:

obtain the intersection between the impls

check if the impl that specializes both parents fits the intersection

pietroalbini · 2018-04-23T10:24:36Z

Ping from triage @giannicic! We haven't heard from you on this in a while, will you have time to address the comments in the future?

nikomatsakis · 2018-04-23T17:56:25Z

@giannicic

My code accept that since A+B+C specializes both A and B but it shouldn't accept it, Right? Because it extends beyond the intersection of A and B. So an additional check is required in order to:

I think it's not so much that the impl should not be accepted as that the program on the whole should not -- that is, the intersection is partly covered, but not completely. I'm actually not entirely sure how to fix that. I guess we basically want to phrase the query:

Does the exist a set of types such that
- impl A applies and impl B applies
- but impl C does not apply

In Chalk it'd certainly be possible to phrase such a query. I have to think a bit about how to phrase it in the current system, and also on the interactions around negative reasoning.

giannicic · 2018-04-26T11:40:20Z

@nikomatsakis
Ok, I understand, let me know if you have any instructions. In the menwhile I'm trying to understand better the trait sistem.
Thanks

shepmaster · 2018-05-06T21:20:10Z

Ping from triage, @giannicic ! Will you have time soon to continue progress on this PR?

giannicic · 2018-05-07T17:23:37Z

@shepmaster
Yes, I'm looking at it. I'm trying to find a way to properly check if the intersection is complete given the impls that overlap.

pietroalbini · 2018-05-14T09:51:23Z

Ping from triage @giannicic! What's your progress on this?

giannicic · 2018-05-14T17:17:36Z

@pietroalbini
Still looking and a bit stucked. I'm trying to phrase a query to the trait system, just to exeperiment on it. I need at least a few weeks to figure it out better and ask for some suggestions by @nikomatsakis

nikomatsakis · 2018-05-15T18:37:40Z

@giannicic I've been wondering how things are going. Perhaps you can pop onto the discord instance and we can schedule a time to chat a bit.

giannicic · 2018-05-16T17:57:37Z

@nikomatsakis
I've joined it. Currently I'm trying to implement a control inside my check_overlap function that should check if the intersection impl is complete following your guidelines here. However at the moment I'm just trying to experiment on the trait system using the two overlapping impls and the intersection impl.
Moreover I'm not able to look at it so often, probably I'll be more free around the 28th, by that time I'll contact you on chat. If there is an urgency to delivering this PR there is no problem for me if you assign it to others. Thanks

nikomatsakis · 2018-05-16T21:52:22Z

@giannicic there is no a great urgency; however, I wonder if it would be better to explore this on the chalk side first. It is (among other things) better equipped to answer questions like "does this exist".

giannicic · 2018-05-18T14:01:05Z

Sure @nikomatsakis.
Let's see it in chalk. I'll contact you on discord around the 28th.
Sorry but I won't be available before that date.

check if the intersection impl is complete

rust-highfive · 2018-05-29T14:37:09Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:53:21] 
[00:53:21] running 1471 tests
[00:53:25] .........................................................................................i..........
[00:53:31] ...............................................i....................................................
[00:53:36] .......F............................................................................................
[00:53:43] ....................................................................................................
[00:53:46] ....................................................................................................
[00:53:51] ....................................................................................................
[00:53:56] ....................................................................................................
[00:53:56] ....................................................................................................
[00:54:00] ....................................................................................................
[00:54:07] .......................................................................................i............
[00:54:12] ................................................................i...................................
[00:54:17] ....................................................................................................
[00:54:23] ....................................................................................................
[00:54:30] .............................................................................................i......
[00:54:33] ...........iiiiiiiii...................................................
[00:54:33] 
[00:54:33] ---- [ui] ui/e0119/issue-28981.rs stdout ----
[00:54:33] diff of stderr:
[00:54:33] 
[00:54:33] 
[00:54:33] - error[E0119]: conflicting implementations of trait `std::ops::Deref` for type `std::cell::Ref<'_, _>`:
[00:54:33] + error[E0119]: conflicting implementations of trait `std::ops::Deref` for type `std::mem::PinMut<'_, _>`:
[00:54:33] 2   --> $DIR/issue-28981.rs:15:1
[00:54:33] 3    |
[00:54:33] 4 LL | impl<Foo> Deref for Foo { } //~ ERROR must be used
[00:54:33] 5    | ^^^^^^^^^^^^^^^^^^^^^^^
[00:54:33] 6    |
[00:54:33] 7    = note: conflicting implementation in crate `core`:
[00:54:33] 7    = note: conflicting implementation in crate `core`:
[00:54:33] -            - impl<'b, T> std::ops::Deref for std::cell::Ref<'b, T>
[00:54:33] +            - impl<'a, T> std::ops::Deref for std::mem::PinMut<'a, T>
[00:54:33] 9              where T: ?Sized;
[00:54:33] 10 
[00:54:33] 11 error[E0210]: type parameter `Foo` must be used as the type parameter for some local type (e.g. `MyStruct<Foo>`)
[00:54:33] 
[00:54:33] The actual stderr differed from the expected stderr.
[00:54:33] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/e0119/issue-28981/issue-28981.stderr
[00:54:33] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/e0119/issue-28981/issue-28981.stderr
[00:54:33] To update references, rerun the tests and pass the `--bless` flag
[00:54:33] To only update this specific test, also pass `--test-args e0119/issue-28981.rs`
[00:54:33] error: 1 errors occurred comparing output.
[00:54:33] status: exit code: 101
[00:54:33] status: exit code: 101
[00:54:33] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/e0119/issue-28981.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/e0119/issue-28981/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/e0119/issue-28981/auxiliary" "-A" "unused"
[00:54:33] ------------------------------------------
[00:54:33] 
[00:54:33] ------------------------------------------
[00:54:33] stderr:
[00:54:33] stderr:
[00:54:33] ------------------------------------------
[00:54:33] {"message":"conflicting implementations of trait `std::ops::Deref` for type `std::mem::PinMut<'_, _>`:","code":{"code":"E0119","explanation":"\nThere are conflicting trait implementations for the same type.\nExample of erroneous code:\n\n```compile_fail,E0119\ntrait MyTrait {\n    fn get(&self) -> usize;\n}\n\nimpl<T> MyTrait for T {\n    fn get(&self) -> usize { 0 }\n}\n\nstruct Foo {\n    value: usize\n}\n\nimpl MyTrait for Foo { // error: conflicting implementations of trait\n                       //        `MyTrait` for type `Foo`\n    fn get(&self) -> usize { self.value }\n}\n```\n\nWhen looking for the implementation for the trait, the compiler finds\nboth the `impl<T> MyTrait for T` where T is all types and the `impl\nMyTrait for Foo`. Since a trait cannot be implemented multiple times,\nthis is an error. So, when you write:\n\n```\ntrait MyTrait {\n    fn get(&self) -> usize;\n}\n\nimpl<T> MyTrait for T {\n    fn get(&self) -> usize { 0 }\n}\n```\n\nThis makes the trait implemented on all types in the scope. So if you\ntry to implement it on another one after that, the implementations will\nconflict. Example:\n\n```\ntrait MyTrait {\n    fn get(&self) -> usize;\n}\n\nimpl<T> MyTrait for T {\n    fn get(&self) -> usize { 0 }\n}\n\nstruct Foo;\n\nfn main() {\n    let f = Foo;\n\n    f.get(); // the trait is implemented so we can use it\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/e0119/issue-28981.rs","byte_start":502,"byte_end":525,"line_start":15,"line_end":15,"column_start":1,"column_end":24,"is_primary":true,"text":[{"text":"impl<Foo> Deref for Foo { } //~ ERROR must be used","highlight_start":1,"highlight_end":24}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"conflicting implementation in crate `core`:\n- impl<'a, T> std::ops::Deref for std::mem::PinMut<'a, T>\n  where T: ?Sized;","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0119]: conflicting implementations of trait `std::ops::Deref` for type `std::mem::PinMut<'_, _>`:\n  --> /checkout/src/test/ui/e0119/issue-28981.rs:15:1\n   |\nLL | impl<Foo> Deref for Foo { } //~ ERROR must be used\n   | ^^^^^^^^^^^^^^^^^^^^^^^\n   |\n   = note: conflicting implementation in crate `core`:\n           - impl<'a, T> std::ops::Deref for std::mem::PinMut<'a, T>\n             where T: ?Sized;\n\n"}
[00:54:33] {"message":"type parameter `Foo` must be used as the type parameter for some local type (e.g. `MyStruct<Foo>`)","code":{"code":"E0210","explanation":"\nThis error indicates a violation of one of Rust's orphan rules for trait\nimplementations. The rule concerns the use of type parameters in an\nimplementation of a foreign trait (a trait defined in another crate), and\nstates that type parameters must be \"covered\" by a local type. To understand\nwhat this means, it is perhaps easiest to consider a few examples.\n\nIf `ForeignTrait` is a trait defined in some external crate `foo`, then the\nfollowing trait `impl` is an error:\n\n```compile_fail,E0210\n# #[cfg(for_demonstration_only)]\nextern crate foo;\n# #[cfg(for_demonstration_only)]\nuse foo::ForeignTrait;\n# use std::panic::UnwindSafe as ForeignTrait;\n\nimpl<T> ForeignTrait for T { } // error\n# fn main() {}\n```\n\nTo work around this, it can be covered with a local type, `MyType`:\n\n```\n# use std::panic::UnwindSafe as ForeignTrait;\nstruct MyType<T>(T);\nimpl<T> ForeignTrait for MyType<T> { } // Ok\n```\n\nPlease note that a type alias is not sufficient.\n\nFor another example of an error, suppose there's another trait defined in `foo`\nnamed `ForeignTrait2` that takes two type parameters. Then this `impl` results\nin the same rule violation:\n\n```ignore (cannot-doctest-multicrate-project)\nstruct MyType2;\nimpl<T> ForeignTrait2<T, MyType<T>> for MyType2 { } // error\n```\n\nThe reason for this is that there are two appearances of type parameter `T` in\nthe `impl` header, both as parameters for `ForeignTrait2`. The first appearance\nis uncovered, and so runs afoul of the orphan rule.\n\nConsider one more example:\n\n```ignore (cannot-doctest-multicrate-project)\nimpl<T> ForeignTrait2<MyType<T>, T> for MyType2 { } // Ok\n```\n\nThis only differs from the previous `impl` in that the parameters `T` and\n`MyType<T>` for `ForeignTrait2` have been swapped. This example does *not*\nviolate the orphan rule; it is permitted.\n\nTo see why that last example was allowed, you need to understand the general\nrule. Unfortunately this rule is a bit tricky to state. Consider an `impl`:\n\n```ignore (only-for-syntax-highlight)\nimpl<P1, ..., Pm> ForeignTrait<T1, ..., Tn> for T0 { ... }\n```\n\nwhere `P1, ..., Pm` are the type parameters of the `impl` and `T0, ..., Tn`\nare types. One of the types `T0, ..., Tn` must be a local type (this is another\norphan rule, see the explanation for E0117). Let `i` be the smallest integer\nsuch that `Ti` is a local type. Then no type parameter can appear in any of the\n`Tj` for `j < i`.\n\nFor information on the design of the orphan rules, see [RFC 1023].\n\n[RFC 1023]: https://github.com/rust-lang/rfcs/blob/master/text/1023-rebalancing-coherence.md\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/e0119/issue-28981.rs","byte_start":502,"byte_end":525,"line_start":15,"line_end":15,"column_start":1,"column_end":24,"is_primary":true,"text":[{"text":"impl<Foo> Deref for Foo { } //~ ERROR must be used","highlight_start":1,"highlight_end":24}],"label":"type parameter `Foo` must be used as the type parameter for some local type","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"only traits defined in the current crate can be implemented for a type parameter","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0210]: type parameter `Foo` must be used as the type parameter for some local type (e.g. `MyStruct<Foo>`)\n  --> /checkout/src/test/ui/e0119/issue-28981.rs:15:1\n   |\nLL | impl<Foo> Deref for Foo { } //~ ERROR must be used\n   | ^^^^^^^^^^^^^^^^^^^^^^^ type parameter `Foo` must be used as the type parameter for some local type\n   |\n   = note: only traits defined in the current crate can be implemented for a type parameter\n\n"}
[00:54:33] {"message":"aborting due to 2 previous errors","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to 2 previous errors\n\n"}
[00:54:33] {"message":"Some errors occurred: E0119, E0210.","code":null,"level":"","spans":[],"children":[],"rendered":"Some errors occurred: E0119, E0210.\n"}
[00:54:33] {"message":"For more information about an error, try `rustc --explain E0119`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about an error, try `rustc --explain E0119`.\n"}
[00:54:33] ------------------------------------------
[00:54:33] 
[00:54:33] thread '[ui] ui/e0119/issue-28981.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3053:9
[00:54:33] note: Run with `RUST_BACKTRACE=1` for a backtrace.
---
[00:54:33] 
[00:54:33] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:54:33] 
[00:54:33] 
[00:54:33] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:54:33] 
[00:54:33] 
[00:54:33] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:54:33] Build completed unsuccessfully in 0:02:55
[00:54:33] Build completed unsuccessfully in 0:02:55
[00:54:33] make: *** [check] Error 1
[00:54:33] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1680d2d8
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

giannicic · 2018-05-29T15:47:28Z

@nikomatsakis
I've updated the PR with an attempt to phrase a query in order to check if the intersection is complete.
With the test that you provided it seems to work correctly but I don't know if is a general solution to the problem.
The relevant code is here

pietroalbini · 2018-06-04T10:24:36Z

Ping from triage @nikomatsakis! This PR needs your review.

TimNN · 2018-06-12T17:36:34Z

Ping from triage @nikomatsakis / @rust-lang/compiler, this PR needs your review.

pietroalbini · 2018-06-18T09:26:50Z

Ping from triage! This PR needs a review, can @nikomatsakis or someone else from @rust-lang/compiler review this?

nikomatsakis · 2018-06-18T19:24:15Z

@giannicic that code is not part of this PR, right? that makes it harder for me to comment on.

nikomatsakis · 2018-06-18T19:36:14Z

There are certainly some flaws in the code. For example, this part:

let param_env1 = tcx.param_env(impl1);
                let param_env2 = tcx.param_env(impl2);

                let mut combined_param_envs_vec =
                    param_env1
                        .caller_bounds
                        .iter()
                        .chain(param_env2.caller_bounds.iter())
                        .map(|p| *p)
.collect::<Vec<_>>();

Those two param_env values are expressed in terms of disjoint sets of variables (one is expressed in terms of the variables of one impl, and one of the variables from the other), so they can't really be combined. We would have to do more careful instantiation of variables. But let's ignore those details for now.

I think that the gist of what you are trying to do is to setup a chalk query where:

using the where clauses from impl A and impl B as hypotheses...
...we try to prove the clauses from impl C

I guess the premise is — if we can prove the clauses from impl C assuming only the clauses from A + B — than there must be "no gap", i.e., no cases uncovered. I think this is .. roughly true but we'll have to do some more sophisticated work. But actually I suspect we want to do this anyway.

We sort of want to phrase the query like

forall<A...> { // parameters from impl A
  forall<B..> { // parameters from impl B
    if (both impls apply) {
      forall<C..> { // parameters from impl C
        impl C applies
      }
    }
  }
}

Basically "if A and B both apply, then C always applies". That seems roughly right but the "both impls apply" bit is tricky. It requires being able to handle things like if (A = B) which -- until now -- we've avoided supporting. I think there is a way to do it that is not too bad, but I've not really worked it out in detail. We should definitely try to find a time to discuss in more detail.

(Also, this condition is perhaps stricter than we want? You could imagine wanting multiple intersection impls that mutually cover the space.)

pietroalbini · 2018-06-25T08:44:30Z

Ping from triage @giannicic! It's been a while since we heard from you, will you have time to work on this again?

nikomatsakis · 2018-06-25T21:58:59Z

My inclination here is probably to close this PR -- but I'd like to capture some of the insights we encountered into a chalk issue to let us discuss the overall design a bit more.

bors · 2018-06-28T11:32:03Z

☔ The latest upstream changes (presumably #51492) made this pull request unmergeable. Please resolve the merge conflicts.

TimNN · 2018-07-03T17:21:51Z

Ping from triage, @nikomatsakis / @giannicic: If I understand correctly this PR can't be merged in its current state and hasn't seen any updates in a while, so I'm closing it. Feel free to re-open in the future.

rust-highfive assigned nikomatsakis Apr 3, 2018

pietroalbini added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 3, 2018

nikomatsakis reviewed Apr 6, 2018

View reviewed changes

nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2018

intersection impls

a5c707a

pietroalbini added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 23, 2018

pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 28, 2018

giannicic force-pushed the intersection_impls branch from dc5f643 to 4726344 Compare May 29, 2018 09:03

Intersection impls

5067f54

check if the intersection impl is complete

giannicic force-pushed the intersection_impls branch from 4726344 to 5067f54 Compare May 29, 2018 13:37

pietroalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 4, 2018

mark-i-m mentioned this pull request Jun 12, 2018

Tracking issue for specialization (RFC 1210) #31844

Open

8 tasks

pietroalbini added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2018

TimNN closed this Jul 3, 2018

TimNN added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 3, 2018

kennytm mentioned this pull request Sep 14, 2019

Need negative trait bound #42721

Open

cowboy-bebug mentioned this pull request Oct 20, 2019

Use specialization feature for AnyTryInto trait cennznet/pact#7

Closed

3 tasks

TimDiekmann mentioned this pull request Aug 5, 2022

Add serde implementation for error-stack hashintel/hash#792

Closed

8 tasks

#48538 Specialization: Intersection Impls #49624

#48538 Specialization: Intersection Impls #49624

Conversation

giannicic commented Apr 3, 2018

TimNN commented Apr 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

giannicic Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis Apr 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietroalbini commented Apr 23, 2018

nikomatsakis commented Apr 23, 2018

giannicic commented Apr 26, 2018

shepmaster commented May 6, 2018

giannicic commented May 7, 2018

pietroalbini commented May 14, 2018

giannicic commented May 14, 2018

nikomatsakis commented May 15, 2018

giannicic commented May 16, 2018

nikomatsakis commented May 16, 2018

giannicic commented May 18, 2018

rust-highfive commented May 29, 2018

giannicic commented May 29, 2018

pietroalbini commented Jun 4, 2018

TimNN commented Jun 12, 2018

pietroalbini commented Jun 18, 2018

nikomatsakis commented Jun 18, 2018

nikomatsakis commented Jun 18, 2018

pietroalbini commented Jun 25, 2018

nikomatsakis commented Jun 25, 2018

bors commented Jun 28, 2018

TimNN commented Jul 3, 2018

giannicic Apr 10, 2018 •

edited

Loading

nikomatsakis Apr 6, 2018 •

edited

Loading