|
| 1 | +- Feature Name: fundamental_attribute |
| 2 | +- Start Date: 2015-03-27 |
| 3 | +- RFC PR: (leave this empty) |
| 4 | +- Rust Issue: (leave this empty) |
| 5 | + |
| 6 | +## Summary |
| 7 | + |
| 8 | +This RFC proposes two rule changes: |
| 9 | + |
| 10 | +1. Modify the orphan rules so that impls of remote traits require a |
| 11 | + local type that is either a struct/enum/trait defined in the |
| 12 | + current crate `LT = LocalTypeConstructor<...>` or a reference to a |
| 13 | + local type `LT = ... | < | &mut LT`. |
| 14 | +2. Restrict negative reasoning so it too obeys the orphan rules. |
| 15 | +3. Introduce an unstable `#[fundamental]` attribute that can be used |
| 16 | + to extend the above rules in select cases (details below). |
| 17 | + |
| 18 | +## Motivation |
| 19 | + |
| 20 | +The current orphan rules are oriented around allowing as many remote |
| 21 | +traits as possible. As so often happens, giving power to one party (in |
| 22 | +this case, downstream crates) turns out to be taking power away from |
| 23 | +another (in this case, upstream crates). The problem is that due to |
| 24 | +coherence, the ability to define impls is a zero-sum game: every impl |
| 25 | +that is legal to add in a child crate is also an impl that a parent |
| 26 | +crate cannot add without fear of breaking downstream crates. A |
| 27 | +detailed look at these problems is |
| 28 | +[presented here](https://gist.github.com/nikomatsakis/bbe6821b9e79dd3eb477); |
| 29 | +this RFC doesn't go over the problems in detail, but will reproduce |
| 30 | +some of the examples found in that document. |
| 31 | + |
| 32 | +This RFC proposes a shift that attempts to strike a balance between |
| 33 | +the needs of downstream and upstream crates. In particular, we wish to |
| 34 | +preserve the ability of upstream crates to add impls to traits that |
| 35 | +they define, while still allowing downstream creates to define the |
| 36 | +sorts of impls they need. |
| 37 | + |
| 38 | +While exploring the problem, we found that in practice remote impls |
| 39 | +almost always are tied to a local type or a reference to a local |
| 40 | +type. For example, here are some impls from the definition of `Vec`: |
| 41 | + |
| 42 | +```rust |
| 43 | +// tied to Vec<T> |
| 44 | +impl<T> Send for Vec<T> |
| 45 | + where T: Send |
| 46 | + |
| 47 | +// tied to &Vec<T> |
| 48 | +impl<'a,T> IntoIterator for &'a Vec<T> |
| 49 | +``` |
| 50 | + |
| 51 | +On this basis, we propose that we limit remote impls to require that |
| 52 | +they include a type either defined in the current crate or a reference |
| 53 | +to a type defined in the current crate. This is more restrictive than |
| 54 | +the current definition, which merely requires a local type appear |
| 55 | +*somewhere*. So, for example, under this definition `MyType` and |
| 56 | +`&MyType` would be considered local, but `Box<MyType>`, |
| 57 | +`Option<MyType>`, and `(MyType, i32)` would not. |
| 58 | + |
| 59 | +Furthermore, we limit the use of *negative reasoning* to obey the |
| 60 | +orphan rules. That is, just as a crate cannot define an impl `Type: |
| 61 | +Trait` unless `Type` or `Trait` is local, it cannot rely that `Type: |
| 62 | +!Trait` holds unless `Type` or `Trait` is local. |
| 63 | + |
| 64 | +Together, these two changes cause very little code breakage while |
| 65 | +retaining a lot of freedom to add impls in a backwards compatible |
| 66 | +fashion. However, they are not quite sufficient to compile all the |
| 67 | +most popular cargo crates (though they almost succeed). Therefore, we |
| 68 | +propose an simple, unstable attribute `#[fundamental]` (described |
| 69 | +below) that can be used to extend the system to accommodate some |
| 70 | +additional patterns and types. This attribute is unstable because it |
| 71 | +is not clear whether it will prove to be adequate or need to be |
| 72 | +generalized; this part of the design can be considered somewhat |
| 73 | +incomplete, and we expect to finalize it based on what we observe |
| 74 | +after the 1.0 release. |
| 75 | + |
| 76 | +### Practical effect |
| 77 | + |
| 78 | +#### Effect on parent crates |
| 79 | + |
| 80 | +When you first define a trait, you must also decide whether that trait |
| 81 | +should have (a) a blanket impls for all `T` and (b) any blanket impls |
| 82 | +over references. These blanket impls cannot be added later without a |
| 83 | +major vesion bump, for fear of breaking downstream clients. |
| 84 | + |
| 85 | +Here are some examples of the kinds of blanket impls that must be added |
| 86 | +right away: |
| 87 | + |
| 88 | +```rust |
| 89 | +impl<T:Foo> Bar for T { } |
| 90 | +impl<'a,T:Bar> Bar for &'a T { } |
| 91 | +``` |
| 92 | + |
| 93 | +#### Effect on child crates |
| 94 | + |
| 95 | +Under the base rules, child crates are limited to impls that use local |
| 96 | +types or references to local types. They are also prevented from |
| 97 | +relying on the fact that `Type: !Trait` unless either `Type` or |
| 98 | +`Trait` is local. This turns out to be have very little impact. |
| 99 | + |
| 100 | +In compiling the libstd facade and librustc, exactly two impls were |
| 101 | +found to be illegal, both of which followed the same pattern: |
| 102 | + |
| 103 | +```rust |
| 104 | +struct LinkedListEntry<'a> { |
| 105 | + data: i32, |
| 106 | + next: Option<&'a LinkedListEntry> |
| 107 | +} |
| 108 | + |
| 109 | +impl<'a> Iterator for Option<&'a LinkedListEntry> { |
| 110 | + type Item = i32; |
| 111 | + |
| 112 | + fn next(&mut self) -> Option<i32> { |
| 113 | + if let Some(ptr) = *self { |
| 114 | + *self = Some(ptr.next); |
| 115 | + Some(ptr.data) |
| 116 | + } else { |
| 117 | + None |
| 118 | + } |
| 119 | + } |
| 120 | +} |
| 121 | +``` |
| 122 | + |
| 123 | +The problem here is that `Option<&LinkedListEntry>` is no longer |
| 124 | +considered a local type. A similar restriction would be that one |
| 125 | +cannot define an impl over `Box<LinkedListEntry>`; but this was not |
| 126 | +observed in practice. |
| 127 | + |
| 128 | +Both of these restrictions can be overcome by using a new type. For |
| 129 | +example, the code above could be changed so that instead of writing |
| 130 | +the impl for `Option<&LinkedListEntry>`, we define a type `LinkedList` |
| 131 | +that wraps the option and implement on that: |
| 132 | + |
| 133 | +```rust |
| 134 | +struct LinkedListEntry<'a> { |
| 135 | + data: i32, |
| 136 | + next: LinkedList<'a> |
| 137 | +} |
| 138 | + |
| 139 | +struct LinkedList<'a> { |
| 140 | + data: Option<&'a LinkedListEntry> |
| 141 | +} |
| 142 | + |
| 143 | +impl<'a> Iterator for LinkedList<'a> { |
| 144 | + type Item = i32; |
| 145 | + |
| 146 | + fn next(&mut self) -> Option<i32> { |
| 147 | + if let Some(ptr) = self.data { |
| 148 | + *self = Some(ptr.next); |
| 149 | + Some(ptr.data) |
| 150 | + } else { |
| 151 | + None |
| 152 | + } |
| 153 | + } |
| 154 | +} |
| 155 | +``` |
| 156 | + |
| 157 | +#### Errors from cargo and the fundamental attribute |
| 158 | + |
| 159 | +We also applied our prototype to all the "Most Downloaded" cargo |
| 160 | +crates as well as the `iron` crate. That exercise uncovered a few |
| 161 | +patterns that the simple rules presented thus far can't handle. |
| 162 | + |
| 163 | +The first is that it is common to implement traits over boxed trait |
| 164 | +objects. For example, the `error` crate defines an impl: |
| 165 | + |
| 166 | +- `impl<E: Error> FromError<E> for Box<Error>` |
| 167 | + |
| 168 | +Here, `Error` is a local trait defined in `error`, but `FromError` is |
| 169 | +the trait from `libstd`. This impl would be illegal because |
| 170 | +`Box<Error>` is not considered local as `Box` is not local. |
| 171 | + |
| 172 | +The second is that it is common to use `FnMut` in blanket impls, |
| 173 | +similar to how the `Pattern` trait in `libstd` works. The `regex` crate |
| 174 | +in particular has the following impls: |
| 175 | + |
| 176 | +- `impl<'t> Replacer for &'t str` |
| 177 | +- `impl<F> Replacer for F where F: FnMut(&Captures) -> String` |
| 178 | +- these are in conflict because this requires that `&str: !FnMut`, and |
| 179 | + neither `&str` nor `FnMut` are local to `regex` |
| 180 | + |
| 181 | +Given that overloading over closures is likely to be a common request, |
| 182 | +and that the `Fn` traits are well-known, core traits tied to the call |
| 183 | +operator, it seems reasonable to say that implementing a `Fn` trait is |
| 184 | +itself a breaking change. (This is not to suggest that there is |
| 185 | +something *fundamental* about the `Fn` traits that distinguish them |
| 186 | +from all other traits; just that if the goal is to have rules that |
| 187 | +users can easily remember, saying that implememting a core operator |
| 188 | +trait is a breaking change may be a reasonable rule, and it enables |
| 189 | +useful patterns to boot -- patterns that are baked into the libstd |
| 190 | +APIs.) |
| 191 | + |
| 192 | +To accommodate these cases (and future cases we will no doubt |
| 193 | +encounter), this RFC proposes an unstable attribute |
| 194 | +`#[fundamental]`. `#[fundamental]` can be applied to types and traits |
| 195 | +with the following meaning: |
| 196 | + |
| 197 | +- A `#[fundamental]` type `Foo` is one where implementing a blanket |
| 198 | + impl over `Foo` is a breaking change. As described, `&` and `&mut` are |
| 199 | + fundamental. This attribute would be applied to `Box`, making `Box` |
| 200 | + behave the same as `&` and `&mut` with respect to coherence. |
| 201 | +- A `#[fundamental]` trait `Foo` is one where adding an impl of `Foo` |
| 202 | + for an existing type is a breaking change. For now, the `Fn` traits |
| 203 | + and `Sized` would be marked fundamental, though we may want to |
| 204 | + extend this set to all operators or some other |
| 205 | + more-easily-remembered set. |
| 206 | + |
| 207 | +The `#[fundamental]` attribute is intended to be a kind of "minimal |
| 208 | +commitment" that still permits the most important impl patterns we see |
| 209 | +in the wild. Because it is unstable, it can only be used within libstd |
| 210 | +for now. We are eventually committed to finding some way to |
| 211 | +accommodate the patterns above -- which could be as simple as |
| 212 | +stabilizing `#[fundamental]` (or, indeed, reverting this RFC |
| 213 | +altogether). It could also be a more general mechanism that lets users |
| 214 | +specify more precisely what kind of impls are reserved for future |
| 215 | +expansion and which are not. |
| 216 | + |
| 217 | +## Detailed Design |
| 218 | + |
| 219 | +### Proposed orphan rules |
| 220 | + |
| 221 | +Given an impl `impl<P1...Pn> Trait<T1...Tn> for T0`, either `Trait` |
| 222 | +must be local to the current crate, or: |
| 223 | + |
| 224 | +1. At least one type must meet the `LT` pattern defined above. Let |
| 225 | + `Ti` be the first such type. |
| 226 | +2. No type parameters `P1...Pn` may appear in the type parameters that |
| 227 | + precede `Ti` (that is, `Tj` where `j < i`). |
| 228 | + |
| 229 | +### Type locality and negative reasoning |
| 230 | + |
| 231 | +Currently the overlap check employs negative reasoning to segregate |
| 232 | +blanket impls from other impls. For example, the following pair of |
| 233 | +impls would be legal only if `MyType<U>: !Copy` for all `U` (the |
| 234 | +notation `Type: !Trait` is borrowed from [RFC 586][586]): |
| 235 | + |
| 236 | +```rust |
| 237 | +impl<T:Copy> Clone for T {..} |
| 238 | +impl<U> Clone for MyType<U> {..} |
| 239 | +``` |
| 240 | + |
| 241 | +[586]: https://github.com/rust-lang/rfcs/pull/586 |
| 242 | + |
| 243 | +This proposal places limits on negative reasoning based on the orphan |
| 244 | +rules. Specifically, we cannot conclude that a proposition like `T0: |
| 245 | +!Trait<T1..Tn>` holds unless `T0: Trait<T1..Tn>` meets the orphan |
| 246 | +rules as defined in the previous section. |
| 247 | + |
| 248 | +In practice this means that, by default, you can only assume negative |
| 249 | +things about traits and types defined in your current crate, since |
| 250 | +those are under your direct control. This permits parent crates to add |
| 251 | +any impls except for blanket impls over `T`, `&T`, or `&mut T`, as |
| 252 | +discussed before. |
| 253 | + |
| 254 | +### Effect on ABI compatibility and semver |
| 255 | + |
| 256 | +We have not yet proposed a comprehensive semver RFC (it's |
| 257 | +coming). However, this RFC has some effect on what that RFC would say. |
| 258 | +As discussed above, it is a breaking change for to add a blanket impl |
| 259 | +for a `#[fundamental]` type. It is also a breaking change to add an |
| 260 | +impl of a `#[fundamental]` trait to an existing type. |
| 261 | + |
| 262 | +# Drawbacks |
| 263 | + |
| 264 | +The primary drawback is that downstream crates cannot write an impl |
| 265 | +over types other than references, such as `Option<LocalType>`. This |
| 266 | +can be overcome by defining wrapper structs (new types), but that can |
| 267 | +be annoying. |
| 268 | + |
| 269 | +# Alternatives |
| 270 | + |
| 271 | +- **Status quo.** In the status quo, the balance of power is heavily |
| 272 | + tilted towards child crates. Parent crates basically cannot add any |
| 273 | + impl for an existing trait to an existing type without potentially |
| 274 | + breaking child crates. |
| 275 | + |
| 276 | +- **Take a hard line.** We could forego the `#[fundamental]` attribute, but |
| 277 | + it would force people to forego `Box<Trait>` impls as well as the |
| 278 | + useful closure-overloading pattern. This seems |
| 279 | + unfortunate. Moreover, it seems likely we will encounter further |
| 280 | + examples of "reasonable cases" that `#[fundamental]` can easily |
| 281 | + accommodate. |
| 282 | + |
| 283 | +- **Specializations, negative impls, and contracts.** The gist |
| 284 | + referenced earlier includes [a section][c] covering various |
| 285 | + alternatives that I explored which came up short. These include |
| 286 | + specialization, explicit negative impls, and explicit contracts |
| 287 | + between the trait definer and the trait consumer. |
| 288 | + |
| 289 | +# Unresolved questions |
| 290 | + |
| 291 | +None. |
| 292 | + |
| 293 | +[c]: https://gist.github.com/nikomatsakis/bbe6821b9e79dd3eb477#file-c-md |
0 commit comments