Partially implement RFC 1647 (`Self` in impl headers) #38920

petrochenkov · 2017-01-08T11:47:11Z

The name resolution part is easy, but the typeck part contains an unexpected problem.

It turns out that Self type depends on bounds and where clauses, so we need to convert them first to determine what the Self type is! If bounds/where clauses can refer to Self then we have a cyclic dependency.
This is required to support impls like this:

// Found in libcollections
impl<I: IntoIterator> SpecExtend<I> for LinkedList<I::Item> { .... }
                                                      ^^^^^ associated type `Item` is found using information from bounds

I'm not yet sure how to resolve this issue.
One possible solution (that feels hacky) is to make two passes over generics - first collect predicates ignoring everything involving Self, then determine Self, then collect predicates again without ignoring anything. (Some kind of lazy on-demand checking or something looks like a proper solution.)

This patch in its current state doesn't solve the problem with Self in bounds, so the only observable things it does is improving error messages and supporting impl Trait<Self> for Type {}.

There's also a question about feature gating. It's non-trivial to detect "newly resolved" Selfs to feature gate them, but it's simple to enable the new resolution behavior when the feature gate is already specified. Alternatively this can be considered a bug fix and merged without a feature gate.

cc #38864
r? @nikomatsakis
cc @eddyb
Whitespace ignoring diff https://github.com/rust-lang/rust/pull/38920/files?w=1

eddyb · 2017-01-08T12:03:31Z

FWIW I had solved the cycle problem a few months back (eddyb@204fdb2) and I'm planning to publish that as the next (and final?) PR after #38813.

However I'm taking a (yet another) detour through doing lifetime elision syntactically (w/ name-resolved definitions but nothing based on type/trait resolution), which would allow a more efficient implementation of my fixpoint algorithm (it's backtracking atm so O(n!)), by making random-access HIR -> Ty conversion possible.

nikomatsakis · 2017-01-17T23:17:09Z

So do we want to wait here for some of @eddyb's work to land instead?

petrochenkov · 2017-01-17T23:25:37Z

@nikomatsakis
Yes, mostly.
This patch still can be landed independently though (it implements the resolve part and therefore supports impl Trait<Self> for Type {} and provides better error messages and tests).

petrochenkov · 2017-01-17T23:26:05Z

I'll rebase this tomorrow.

petrochenkov · 2017-01-18T20:50:32Z

Rebased.

nikomatsakis · 2017-01-25T17:28:48Z

r? @eddyb -- he's been in this area so is best person to review

eddyb · 2017-01-25T17:56:13Z

@bors r+

bors · 2017-01-25T17:56:14Z

📌 Commit f9bdf34 has been approved by eddyb

bors · 2017-01-25T23:08:16Z

⌛ Testing commit f9bdf34 with merge df8debf...

@nikomatsakis

Partially implement RFC 1647 (`Self` in impl headers) The name resolution part is easy, but the typeck part contains an unexpected problem. It turns out that `Self` type *depends* on bounds and `where` clauses, so we need to convert them first to determine what the `Self` type is! If bounds/`where` clauses can refer to `Self` then we have a cyclic dependency. This is required to support impls like this: ``` // Found in libcollections impl<I: IntoIterator> SpecExtend<I> for LinkedList<I::Item> { .... } ^^^^^ associated type `Item` is found using information from bounds ``` I'm not yet sure how to resolve this issue. One possible solution (that feels hacky) is to make two passes over generics - first collect predicates ignoring everything involving `Self`, then determine `Self`, then collect predicates again without ignoring anything. (Some kind of lazy on-demand checking or something looks like a proper solution.) This patch in its current state doesn't solve the problem with `Self` in bounds, so the only observable things it does is improving error messages and supporting `impl Trait<Self> for Type {}`. There's also a question about feature gating. It's non-trivial to *detect* "newly resolved" `Self`s to feature gate them, but it's simple to *enable* the new resolution behavior when the feature gate is already specified. Alternatively this can be considered a bug fix and merged without a feature gate. cc #38864 r? @nikomatsakis cc @eddyb Whitespace ignoring diff https://github.com/rust-lang/rust/pull/38920/files?w=1

bors · 2017-01-26T01:50:36Z

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing df8debf to master...

eddyb · 2017-02-02T12:02:08Z

src/librustc_typeck/astconv.rs

-                    ty.subst(tcx, free_substs)
+
+                // FIXME: Self type is not always computed when we are here because type parameter
+                // bounds may affect Self type and have to be converted before it.


I was working on the on-demand stuff and I just realized this can be made to work with AstConv::get_item_type.
In all fairness, ItemImpl is not hooked up to that in rustc_typeck::collect because of predicate complications, that are fixed on this branch I'm playing with (even if you can't get it from tcx.item_type, that's not needed here).

Version 1.16.0 (2017-03-16) =========================== Language -------- * Lifetimes in statics and consts default to `'static`. [RFC 1623] * [The compiler's `dead_code` lint now accounts for type aliases][38051]. * [Uninhabitable enums (those without any variants) no longer permit wildcard match patterns][38069] * [Clean up semantics of `self` in an import list][38313] * [`Self` may appear in `impl` headers][38920] * [`Self` may appear in struct expressions][39282] Compiler -------- * [`rustc` now supports `--emit=metadata`, which causes rustc to emit a `.rmeta` file containing only crate metadata][38571]. This can be used by tools like the Rust Language Service to perform metadata-only builds. * [Levenshtein based typo suggestions now work in most places, while previously they worked only for fields and sometimes for local variables][38927]. Together with the overhaul of "no resolution"/"unexpected resolution" errors (#[38154]) they result in large and systematic improvement in resolution diagnostics. * [Fix `transmute::<T, U>` where `T` requires a bigger alignment than `U`][38670] * [rustc: use -Xlinker when specifying an rpath with ',' in it][38798] * [`rustc` no longer attempts to provide "consider using an explicit lifetime" suggestions][37057]. They were inaccurate. Stabilized APIs --------------- * [`VecDeque::truncate`] * [`VecDeque::resize`] * [`String::insert_str`] * [`Duration::checked_add`] * [`Duration::checked_sub`] * [`Duration::checked_div`] * [`Duration::checked_mul`] * [`str::replacen`] * [`str::repeat`] * [`SocketAddr::is_ipv4`] * [`SocketAddr::is_ipv6`] * [`IpAddr::is_ipv4`] * [`IpAddr::is_ipv6`] * [`Vec::dedup_by`] * [`Vec::dedup_by_key`] * [`Result::unwrap_or_default`] * [`<*const T>::wrapping_offset`] * [`<*mut T>::wrapping_offset`] * `CommandExt::creation_flags` * [`File::set_permissions`] * [`String::split_off`] Libraries --------- * [`[T]::binary_search` and `[T]::binary_search_by_key` now take their argument by `Borrow` parameter][37761] * [All public types in std implement `Debug`][38006] * [`IpAddr` implements `From<Ipv4Addr>` and `From<Ipv6Addr>`][38327] * [`Ipv6Addr` implements `From<[u16; 8]>`][38131] * [Ctrl-Z returns from `Stdin.read()` when reading from the console on Windows][38274] * [std: Fix partial writes in `LineWriter`][38062] * [std: Clamp max read/write sizes on Unix][38062] * [Use more specific panic message for `&str` slicing errors][38066] * [`TcpListener::set_only_v6` is deprecated][38304]. This functionality cannot be achieved in std currently. * [`writeln!`, like `println!`, now accepts a form with no string or formatting arguments, to just print a newline][38469] * [Implement `iter::Sum` and `iter::Product` for `Result`][38580] * [Reduce the size of static data in `std_unicode::tables`][38781] * [`char::EscapeDebug`, `EscapeDefault`, `EscapeUnicode`, `CaseMappingIter`, `ToLowercase`, `ToUppercase`, implement `Display`][38909] * [`Duration` implements `Sum`][38712] * [`String` implements `ToSocketAddrs`][39048] Cargo ----- * [The `cargo check` command does a type check of a project without building it][cargo/3296] * [crates.io will display CI badges from Travis and AppVeyor, if specified in Cargo.toml][cargo/3546] * [crates.io will display categories listed in Cargo.toml][cargo/3301] * [Compilation profiles accept integer values for `debug`, in addition to `true` and `false`. These are passed to `rustc` as the value to `-C debuginfo`][cargo/3534] * [Implement `cargo --version --verbose`][cargo/3604] * [All builds now output 'dep-info' build dependencies compatible with make and ninja][cargo/3557] * [Build all workspace members with `build --all`][cargo/3511] * [Document all workspace members with `doc --all`][cargo/3515] * [Path deps outside workspace are not members][cargo/3443] Misc ---- * [`rustdoc` has a `--sysroot` argument that, like `rustc`, specifies the path to the Rust implementation][38589] * [The `armv7-linux-androideabi` target no longer enables NEON extensions, per Google's ABI guide][38413] * [The stock standard library can be compiled for Redox OS][38401] * [Rust has initial SPARC support][38726]. Tier 3. No builds available. * [Rust has experimental support for Nvidia PTX][38559]. Tier 3. No builds available. * [Fix backtraces on i686-pc-windows-gnu by disabling FPO][39379] Compatibility Notes ------------------- * [Uninhabitable enums (those without any variants) no longer permit wildcard match patterns][38069] * In this release, references to uninhabited types can not be pattern-matched. This was accidentally allowed in 1.15. * [The compiler's `dead_code` lint now accounts for type aliases][38051]. * [Ctrl-Z returns from `Stdin.read()` when reading from the console on Windows][38274] * [Clean up semantics of `self` in an import list][38313] [37057]: rust-lang/rust#37057 [37761]: rust-lang/rust#37761 [38006]: rust-lang/rust#38006 [38051]: rust-lang/rust#38051 [38062]: rust-lang/rust#38062 [38062]: rust-lang/rust#38622 [38066]: rust-lang/rust#38066 [38069]: rust-lang/rust#38069 [38131]: rust-lang/rust#38131 [38154]: rust-lang/rust#38154 [38274]: rust-lang/rust#38274 [38304]: rust-lang/rust#38304 [38313]: rust-lang/rust#38313 [38314]: rust-lang/rust#38314 [38327]: rust-lang/rust#38327 [38401]: rust-lang/rust#38401 [38413]: rust-lang/rust#38413 [38469]: rust-lang/rust#38469 [38559]: rust-lang/rust#38559 [38571]: rust-lang/rust#38571 [38580]: rust-lang/rust#38580 [38589]: rust-lang/rust#38589 [38670]: rust-lang/rust#38670 [38712]: rust-lang/rust#38712 [38726]: rust-lang/rust#38726 [38781]: rust-lang/rust#38781 [38798]: rust-lang/rust#38798 [38909]: rust-lang/rust#38909 [38920]: rust-lang/rust#38920 [38927]: rust-lang/rust#38927 [39048]: rust-lang/rust#39048 [39282]: rust-lang/rust#39282 [39379]: rust-lang/rust#39379 [`<*const T>::wrapping_offset`]: https://doc.rust-lang.org/std/primitive.pointer.html#method.wrapping_offset [`<*mut T>::wrapping_offset`]: https://doc.rust-lang.org/std/primitive.pointer.html#method.wrapping_offset [`Duration::checked_add`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_add [`Duration::checked_div`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_div [`Duration::checked_mul`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_mul [`Duration::checked_sub`]: https://doc.rust-lang.org/std/time/struct.Duration.html#method.checked_sub [`File::set_permissions`]: https://doc.rust-lang.org/std/fs/struct.File.html#method.set_permissions [`IpAddr::is_ipv4`]: https://doc.rust-lang.org/std/net/enum.IpAddr.html#method.is_ipv4 [`IpAddr::is_ipv6`]: https://doc.rust-lang.org/std/net/enum.IpAddr.html#method.is_ipv6 [`Result::unwrap_or_default`]: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or_default [`SocketAddr::is_ipv4`]: https://doc.rust-lang.org/std/net/enum.SocketAddr.html#method.is_ipv4 [`SocketAddr::is_ipv6`]: https://doc.rust-lang.org/std/net/enum.SocketAddr.html#method.is_ipv6 [`String::insert_str`]: https://doc.rust-lang.org/std/string/struct.String.html#method.insert_str [`String::split_off`]: https://doc.rust-lang.org/std/string/struct.String.html#method.split_off [`Vec::dedup_by_key`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.dedup_by_key [`Vec::dedup_by`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.dedup_by [`VecDeque::resize`]: https://doc.rust-lang.org/std/collections/vec_deque/struct.VecDeque.html#method.resize [`VecDeque::truncate`]: https://doc.rust-lang.org/std/collections/vec_deque/struct.VecDeque.html#method.truncate [`str::repeat`]: https://doc.rust-lang.org/std/primitive.str.html#method.repeat [`str::replacen`]: https://doc.rust-lang.org/std/primitive.str.html#method.replacen [cargo/3296]: rust-lang/cargo#3296 [cargo/3301]: rust-lang/cargo#3301 [cargo/3443]: rust-lang/cargo#3443 [cargo/3511]: rust-lang/cargo#3511 [cargo/3515]: rust-lang/cargo#3515 [cargo/3534]: rust-lang/cargo#3534 [cargo/3546]: rust-lang/cargo#3546 [cargo/3557]: rust-lang/cargo#3557 [cargo/3604]: rust-lang/cargo#3604 [RFC 1623]: https://github.com/rust-lang/rfcs/blob/master/text/1623-static.md

rust-highfive assigned nikomatsakis Jan 8, 2017

petrochenkov changed the title ~~[WIP] Implement RFC 1647 (Self in impl headers)~~ Partially implement RFC 1647 (Self in impl headers) Jan 17, 2017

petrochenkov force-pushed the selfimpl branch from 4b8e39b to 939368d Compare January 18, 2017 20:50

Resolve Self in impl headers

f9bdf34

petrochenkov force-pushed the selfimpl branch from 939368d to f9bdf34 Compare January 21, 2017 09:31

rust-highfive assigned eddyb and unassigned nikomatsakis Jan 25, 2017

bors merged commit f9bdf34 into rust-lang:master Jan 26, 2017

bluss added the relnotes Marks issues that should be documented in the release notes of the next release. label Jan 28, 2017

eddyb reviewed Feb 2, 2017

View reviewed changes

petrochenkov mentioned this pull request Feb 4, 2017

ICE when referencing associated type via Self in where clauses #39535

Closed

petrochenkov deleted the selfimpl branch March 16, 2017 19:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Partially implement RFC 1647 (`Self` in impl headers) #38920

Partially implement RFC 1647 (`Self` in impl headers) #38920

petrochenkov commented Jan 8, 2017 •

edited

Loading

eddyb commented Jan 8, 2017 •

edited

Loading

nikomatsakis commented Jan 17, 2017

petrochenkov commented Jan 17, 2017 •

edited

Loading

petrochenkov commented Jan 17, 2017

petrochenkov commented Jan 18, 2017

nikomatsakis commented Jan 25, 2017

eddyb commented Jan 25, 2017

bors commented Jan 25, 2017

bors commented Jan 25, 2017

bors commented Jan 26, 2017

eddyb Feb 2, 2017

Partially implement RFC 1647 (Self in impl headers) #38920

Partially implement RFC 1647 (Self in impl headers) #38920

Conversation

petrochenkov commented Jan 8, 2017 • edited Loading

eddyb commented Jan 8, 2017 • edited Loading

nikomatsakis commented Jan 17, 2017

petrochenkov commented Jan 17, 2017 • edited Loading

petrochenkov commented Jan 17, 2017

petrochenkov commented Jan 18, 2017

nikomatsakis commented Jan 25, 2017

eddyb commented Jan 25, 2017

bors commented Jan 25, 2017

bors commented Jan 25, 2017

bors commented Jan 26, 2017

eddyb Feb 2, 2017

Choose a reason for hiding this comment

Partially implement RFC 1647 (`Self` in impl headers) #38920

Partially implement RFC 1647 (`Self` in impl headers) #38920

petrochenkov commented Jan 8, 2017 •

edited

Loading

eddyb commented Jan 8, 2017 •

edited

Loading

petrochenkov commented Jan 17, 2017 •

edited

Loading