-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Orphan rules are stricter than we would like #1856
Comments
The solution I like to the third scenario is to enable a greater variety of blanket impls through specialization & mutual exclusion. For example, if there is a Traits like This has pretty bad limitations when it comes to things like ndarray and rayon because the high performance expectation probably necessitates some sort of API access that wouldn't be contained in that ancestor trait (possibly even some unsafe code). |
this doesn't work, because we'd also want to have impl<T> Serialize for T where T: Map We already ran into this when we tried to impl<I, T> Serialize for I where I: Iterator<Item = T>, T: Serialize |
@oli-obk It does work with mutual exclusion, by making it incoherent for a single type to implement both |
I've mused a kind of lattice system where if one uses crate The motivation here is sort of a collaborative look at the expression problem: It's sort of arbitrary to say whether the trait or type should own the impls when its a joint endeavor. The traditional approach to the expression problem was about allowing everyone to work in isolation, which is what made it hard/impossible to resolve, here the idea is since the author of the type and the trait both have a stake, they both should be able to write the impl. Ideally this would be coupled with a jointly-owned crate on crates.io they both could publish. I don't have a detailed design or informal argument that this is sound, but as this has long been my rough thinking on the problem from a software engineering perspective, and I figured I should share it. For the String case (outputs not inputs more broadly), core should probably say "I defer to a crate called collections" giving it a non-orphan exception, along with maybe an input signature the "deferred impl" must obey (for purposes of core being able to define other blankets). |
I'm in favor of a boundary that isn't a crate boundary and contains multiple crates but is logically the same "project", where a different set of rules apply etc. I have never really pushed for this but I have often talked about separating the notion of a package from the notion of a crate. However, this may only further muddle our already-confusing crate/mod system. |
I'd like such a wider boundary, a way for a parent crate to delegate to other crates, so that they are permitted to implement traits for types in the parent crate “in its stead”. Something like this: [package] Now |
I would rather have a way for a child crate, of two parent crates, to define interop code. This means that
that was a lot of "interop"s. |
FWIW the conditional features rfc eases the interop use case (but doesn't solve it completely) |
@Manishearth and I talked about orphan issues tonight, and he gave a good example from servo, which I admit I may have badly misunderstood. But based on my understanding, I discovered a pattern for 'smuggling' code for foreign types. There are some restrictions on this pattern, but it seems quite powerful. An example is FFI conversion. Maybe you have a single crate which defines all of your FFI types (possibly in an automatically generated fashion), and its shared by many other crates in your project. So you want all your FFI types to implement from/into their real types, and for your real types to implement from/into your FFI types. This runs into orphan rules because your FFI library doesn't know about the real types, because its a shared dependency of all the crates containing the real types. Here's a way of defining those traits (and a 'smuggler' impl) which totally compiles and works (and it blows my mind): trait Real: Sized {
type Ffi: Ffi<Self>;
fn from_ffi(source: Self::Ffi) -> Self;
fn into_ffi(self) -> Self::Ffi;
}
trait Ffi<T>: Sized {
fn into_real(self) -> T;
fn from_real(real: T) -> Self;
}
impl<T> Ffi<T> for T::Ffi where T: Real {
fn into_real(self) -> T {
T::from_ffi(self)
}
fn from_real(real: T) -> T::Ffi {
real.into_ffi()
}
} You can implement the I was doubtful that this could be coherent, but because You can actually smuggle arbitrary code (not only conversions) into the FFI crate by just having it delegate to a function on the |
Could you please rephrase this, or maybe with example code? I confess I don't understand at all what each "this" and "another" is referring to. |
The automatic features RFC (did you mean this one @Manishearth?) seems like it doesn't solve any of the hard issues around either version coupling or orphan rules. It is like a convenience around what we can already achieve, while we want something that really bends the current rules a bit. |
yep. used to be called "conditional dependencies", now is "automatic features", mixed the names.
Yeah, it doesn't, but it smoothens some of the more common annoying use cases. |
@glaebhoerl sorry, did a sloppy job writing on my phone. I've gone back and cleaned up that post so it should be understandable now. |
I wonder if you would consider a way to make binary operator traits like this work:
It would be a much cleaner way of implementing these operators for numerical programming. |
Isn't there a case that binary operators should've been defined on 2-tuples? Ala I donno if this could changed in a backward compatible way with a blanket |
You've literally just decomposed the problem into figuring out the exact same predicate, but specifically for tuples. It's good to have the two use the same rules, but you don't have a simpler problem to solve. |
Would like to have something like this allowed: impl ForeignTrait2 for ForeignWrapper<MyStruct> { ... } Currently results in |
Are there any caveats I'm not considering for completely ignoring the orphan rule for BINARY crates? For example: I'd love to be able to |
afaik permitting leaf crates to completely ignore the orphan rules would be perfectly sound and not create any of the subtle composability issues we usually talk about on issues like this. The concerns there would be about it preventing you from easily refactoring the leaf/binary code into a non-leaf library, or creating a surprisingly huge and fundamental difference between what is allowed in libraries versus binaries that's especially likely to catch newcomers off guard. In particular, I'm told it's common to design Rust executables as mostly a library crate with only a very thin wrapper in the actual binary crate, in which case binary-only powers wouldn't help much. |
I think the part about catching newcomers off guard can be helped with good error messages, and warnings (such as a link to what the orphan rule is, and why it's needed for library crates, also explain that this limitation does not hold for binary crates, and why that is so). As far as I see - although I still consider myself a newcomer to rust - is that the habit of having a small wrapper as the binary and having most of the code in a library is getting less common, especially since cargo now defaults to binary crates. Never really understood what was the point of that, I really think it's a fallacy to think that a full-blown application would/(or even should!) be easily converted to a library, other than some very small tools. And about introducing a fundamental difference between them.... Yeah, I totally see your point there, however the use cases for these crates are fundamentally different (one is for reuse in other components, the other is for distributing and running), so I'm not convinced this is a bad thing. And allowing it gives us a lot of expressiveness in return, which in some cases cannot be worked around (such as So all in IMHO the orphan rule was introduced as a "necessary limitation" to circumvent the composability issues, however it's a limitation that I would think is not needed for binary crates. |
The orphan rules are required for binary crates in order to keep semver working correctly, as an application is not necessarily distributed with its (also, I have noticed that until now every time I tried designing an application as a library wrapped by a binary I actually somehow managed it, and it makes for much cleaner code IMO as libraries make coupling and dependencies much more explicit) |
Well to me that seems like the cause of the incompatibility with semver is not really caused by not enforcing the orphan rule, but by not distributing the Cargo.lock, which is a very questionable thing to do - basically that means you trust every dependency (direct and transitive) to adhere to semver, which I think everyone already realized that unfortunately there are and always will be people, who don't. Also there's this: https://gist.github.com/jashkenas/cbd2b088e20279ae2c8e, which I don't agree with in full extent, but there are some very interesting points to consider. Furthermore, I don't think we should restrict the experessiveness of a language in order to "be compatible with" an arbitrarily chosen versioning scheme, unless it is established, that it is a convention that all the rust things are to follow that versioning scheme. We can issue warnings that hey, here be lions, be careful, we can put this behind a special keyword / compiler flag / rustc cmdline argument / whatever, but the amount of help we can get while debugging or just extending stuff without having to fork every single dependency I just want to make Serializeable or Debuggable is hard to ignore. |
@axos88 The fundamental problem isn't cargo and semver of third party library crates, but the versioning of Rust itself. We've made backward compatibility guarantees regarding the standard library that would mean we wouldn't be able to add impls to it if orphan impls were allowed, since someone could have written than impl as an orphan impl. |
This really needs to be fixed instead of pestering crate owners to remember to implement/derive Debug all the time... |
@withoutboats That could be circumvented, if we'd say that orphan impls always override default impls. |
@axos88 I think just refuse to compile is good enough. In many package managers, they have multiple packages provide the same functionality, when installing both, a conflict will occur and the package manager will ask the user to choose either one. Here, if the user want to use new crate with additional impls and keep the homemade impls, the compiler panics! |
@cloudhan that's not a good way because of the reasons @withoutboats described above. An application may stop compiling only because of a seemingly non-breaking change in rust, or another dependency, which is not acceptable for obvious reasons. However if we specify a clear overriding order (and possibly still issue compiler warnings, so they don't go unnoticed, perhaps unless somebody clearly describes that they WANT to override), things won't break. |
overriding violates the fundamental property of coherence we are trying to maintain - upstream code may already have been compiled using the impl which you have overriden, meaning that the same type impl is dispatched differently in different parts of your program. |
As a rule, you could never add |
impl Trait for MyType = delete maybe better than simply left the trait unimplemented and implicitly imply it. |
I'd certainly support adding comments about deliberately unimplemented traits, but I don't see any benefits to adding a language-level syntax for it (C++ has similar syntax because, unlike Rust, it has implicitly provided methods and method overriding). |
You cannot simply ban specific traits like It's only auto-traits like Send and Sync that developers must unimplement now, which although annoying only comes up in niche cases, and people can reasonable learn about. We communicate performance characteristics with missing impls too: You do algebra on elliptic curve points in projective coordinates, but you cannot hash or order them without converting to affine coordinates, which takes time. If you permit adding comparison operators, then you'll wind up with folks writing amazingly slow hash and btree maps. Instead, we should add a mechanism for trait warnings because we do want Just fyi, these trait warnings would not suffice for my previous use cases because say cloning a session type for a multi-signature will compromise the user's private key, not just enable some DoS attack. |
Random spitball I thought of and figured I'd mention here, though it has a big problem: what if we explicitly allowed orphans in designated crates, to bridge compatibility gaps? Succintly, we have a rule that when crates In my view, the restriction I mentioned should be viewed as a problem in and of itself, as it's imposing maintenance burden on the author of the smart pointer library, even small maintenance burden, for users to use it in conjunction with another library. The only available workaround is a wrapper type with its massive usability hit, which sucks. And the author may not implement it, because they are too busy, away, burned out, etc. Or maybe they just hate the trait and refuse to implement it. And the bigger the ecosystem of useful utility traits grows, the worse the problem gets. We don't want to reach a point where a library can't be considered useful until it implements all of 25 different utility crates' traits, with a feature flag for each. Additionally, workarounds currently in use, particularly feature flags, may work well when there's a clear dependency ordering of "coreness" of libraries. This problem only gets worse when there are multiple types in the traits, such as A concrete idea to address this problem is that a crate can have a mapping of crates for which it specifies it provides compatibility, and this relaxes orphan rules. For instance, suppose that The effect of saying that a crate provides compatibility between a set of crates is that it is permitted to write instances for a trait defined in one of the crates, provided that it meets the local type requirements for all of the other crates. For instance, in the case of
In the case of There are three rules to be followed:
This maintains coherence because the only instances allowed are those that would require a dependency relationship between the named crates that must be forbidden by rule 2. Some issues mentioned upthread do not arise here: libraries are not restricted in adding implementations, because a library can still not add single-crate orphan instances, and there's no risk of ordering issues in compile because the rules are enforced before the compile begins and guarantee coherence when they are followed. I think there's some fuziness about the common dependency permission (and specifically, what if another crate implements compatibility for all of the crates and the common dependency), but it could be removed if it can't be made to work. These rules can probably be relaxed, for instance to allow implementations of There's a minor interaction with semver that ought to be noted here: since different major versions of the same crate are treated as different crates altogether, there's some ambiguity. The easiest resolution is simply to make it a hard rule that a crate that Unfortunately, there are two major limitations to this approach, both of which present serious difficulties. First, collisions when multiple compatibility crates exist could damage and fragment the ecosystem. Second, the semver implications on trait authors may be too much of a problem. The issue with collisions is a particularly nasty dependency hell, where two different libraries might choose to depend on different crates offering compatibility between two other crates. These libraries cannot be used together. The problem isn't entirely unique, as there are definitely instances in the wild of competing crates, and the frustrations caused by trying to mix packages that made different choices. But these situations, although frustrating, can be worked around. Collisions between compatibility crates, on the other hand, don't seem to admit workarounds. One approach we might consider would be the use of overrides in the top-level The issue with semver is that the relaxations to the coherence rules only work if the principal crate cannot add those instances themselves. In an individual compile, ensuring an absence of a dependency relationship is enough for this, but the implication there is that adding a dependency becomes a breaking change! I had originally thought that this would be limited to direct dependencies, and might be a little bit more workable perhaps, but unfortunately it's not. Because of type aliases and re-exports, it's possible for an instance to be defined on an indirect dependency. Thus, depending on another package which added a dependency could be enough for a breaking change. Maybe there's a kernel of workability in here somewhere, but I don't immediately see it and this comment is long enough already. |
It's definitely come up before. My preferred nickname for this family of solutions is "official orphans", and I've previously attempted to gather some relevant discussion links on Ixrec/rust-orphan-rules#7 in the hopes of defragmenting the broader conversation. I believe we can categorically rule out any proposal that only solves "first-order orphans" by allowing higher-order orphans to break coherence / cause dependency hells (because if we're willing to break coherence, it'd be much simpler to just allow all orphans). That immediately implies any official orphans proposal must have the "non-parent" crates explicitly declare which crates may provide which orphan impls (and the toolchain validate those declarations are overlap-free), so that there can never be conflicting orphans in the ecosystem. The semver details you discuss would certainly have to be part of any concrete proposal, but imo they're probably solvable (if we avoid thinking about any first-order-only "solutions"), and not the blocking issue anyway. AFAICT the reason no one's pursuing "official orphan" solutions is simply that core crate maintainers don't want to add official orphan lists/declarations to their maintenance burden. I believe I've seen @sgrif express this in a few past discussions. |
I mean, I'm of the opinion that coherence was a mistake, at the very least for compile time :P. But yeah, fundamentally there is a social problem here, and it sucks that there isn't a good technical way we've found to at least reduce the issues. Not that we can fully eliminate them---there will always be examples of some crates just refusing to work together in any reasonable manner and that's okay. I think it's all about isolating them. I'm also a bit skeptical that you can safely assume uniqueness across the entire ecosystem, when there is more than one of those too. The other thing I could think of is that while dependency hell can exist, and overriding might genuinely be a problem, maybe we can contain them to the point where they're unlikely to cause major damage. For instance, the following seem likely true to me:
If these both hold, then maybe we can manage to get away with some additional restrictions, such as a crate being able to explicitly declare which orphan implementations it wants to have exist, and can this asserts a contract that that crate is okay with substitute impls that meet the trait contract. Then we allow overrides as I discussed above, and this would work because the crate has promised it is okay with either option. We could also expect that orphan crates try to limit their scope so that there's less collision in the ecosystem (possibly enforced with restrictions on e.g. exporting non-impl items, in a manner similar to proc macro crates). |
I think the automatic features proposal linked above still sounds by far the most promising. In particular, there is nothing specific to traits about this problem: You could need an inherent method, wrapper type, type alias, mixed trait impl, macros, etc. that requires another crate, while you avoid actually depending upon that crate. There is certainly an elegance in neither chrono nor diesel declaring the other to be higher level, by chrono_diesel being their cross over, but any advantage is lost once authors force everything into traits unnecessarily. I think any chrono_diesel solution only really becomes interesting when crates delegate all their powers, including privileged visibility, so like chrono_diesel could handle sealed traits and private macros from both chrono and diesel together. At that point, chrono_diesel cannot "exist completely independently in the ecosystem" but becomes an integral part of both. |
Just for completeness: my current opinion is that insisting on coherence is a big win, and we can "solve" these problems (to the extent that it's possible and worthwhile to solve them at all) through several indirectly related proposals with independent motivation. The big ones being:
I strongly suspect that when all of these are completed, the orphan rules will be a small enough pain point that no escape hatches or other direct weakening of the rules will be needed, with one exception. But there's no good way to evaluate that claim until these make significantly more progress, so there's probably not much else to say at this level. IMO the "official orphans" like chrono_diesel are the most interesting cases precisely because they are that one exception, the one big pain point in the whole design space that none of these other proposals really help with. But I still think that's because--as @burdges said--"existing completely independently" just isn't possible without giving up coherence (this post appears to disagree, but IIUC what it's proposing is yet another "first-order only" solution), hence we're stuck on the social problem there. |
I disagree quite strongly. There's no reason that an impl crate like a hypothetical chrono_diesel should need access to private functionality from either chrono or from diesel. All functionality should be already a part of the public interfaces of those crates, and the traits implemented elsewhere only glue them together. Your proposal makes the problem worse, by requiring both chrono and diesel authors to work together. This is unworkable; @dtolnay is not going to sign off on every single package that wants to implement serde traits, for instance. It also has serious problems for unsafe code: if we can violate visibility, then suddenly unsafe code that relies on things not being accessed from outside the crate (or even from other modules within the crate) can't make these assumptions any more, and the proof guarantees can't be met. This would force all the crates involved to be evaluated with respect to each other to verify safety guarantees and to release in lockstep.
Automatic features improves usability, but does nothing to solve the problem of a linear number of maintainers having to maintain a quadratic number of instances. Specialization and mutually exclusive traits, I may be missing something, but I don't see how they help with this problem. If you mean that they help with some of the other problems in the first comment, I agree there, but those are really different problems. Delegation would significantly ease on the level of boilerplate required to create a wrapper type, but it still brings with it ugly wrapping/unwrapping to convert between the various wrapped versions. It's a big help, but it's a workaround and not a solution. As for coherence, while I think it was the wrong choice for the language to require coherence to begin with, I'm skeptical that Rust really has the capacity to accept a relaxation of coherence without causing other issues. I do think that we can insist on coherence easily with official orphans, as I said above, but at the cost of dependency hell, and I think that trying to find workarounds to that is possibly more fruitful than relaxing coherence. But I might be wrong here. P.S. I don't know what you mean by "first-order". Could you define it for me? Thanks! |
Forgot to reply to this---if I understand what it is proposing, it's that a crate can "cheat' the orphan rule by using a potentially coherence-violating instance internally. As long as it doesn't expose anything to any other crate, then a collision can't be visible and thus there's no issues with coherence. I thought there was an RFC proposing to do this for inherent impls (which are much simpler because neither of the below problems apply) but I can't seem to find it. The obvious and immediate restriction is that you can't create a trait object from such an impl, or at least you can't let it escape your crate, because then you have a different vtable pointer and so you'll look like a different type. The more subtle and nasty problem is the interaction with generics in the crate. If I have a private implementation of, say |
There is afaik no reason for chrono_diesel to exist since diesel with an automatic chrono feature handles everything far better. Automatic features are both a far more conservative and vastly more useful. Automatic features cannot provide access to private items from both sides however, so I'm only highlighting their one real limitation, especially if both sides employ sealed traits.
If either side's public interface suffices, then automatic features are far better than another microcrate that breaks rustdoc, requires using traits when inherent items fit better, etc. I never suggested chrono_diesel as an alternative to automatic features, and I'd argue against chrono_diesel even with privileged visibility, but I think the privileged visibility variant sounds like the interesting case.
There are many harmful or questionable impls, like say HashMap needs to be Clone but this causes security bugs. At least automatic features place blame firmly onto one project. I've zero faith in any community managing literally quadratic In particular, I'd expect tiny ad hoc (non-permission/workspaced/etc) joiner crates would become a bug minefield with "maintainers" who often either do not care about miss-use or else abandon them preventing fixes. And good luck winning the crate name chrono_diesel from the version with an sql injection exploit whose maintainer disappears or whatever. As an aside, we need "impl warnings" so that iffly impls like
I've sent enough pull request adding core traits to types in third party crates, but I only rarely want a third party trait defined for a type from a third party crate. There are third party crates like serde designed well enough to warrant this, but among those many like rand rarely fit external types. And most crates' traits prove too bespoke without some wrapper type, so.. It's actually delegation that addresses the most common case! |
That feature exists as a workaround only. It should not have to live in Diesel |
The current orphan rules (described as "covered first" in my blog post) are too restrictive in some cases. The purpose of this RFC issue is to catalog cases where the rules are tighter than we would like and to also point at possible solutions.
Scenario. Type parameters restrict our flexibility
In the discussion around the try trait in #1718, we realized that the orphan rules would prohibit one from adding an impl like this:
This is sort of annoying because if there were no type parameters involved, the impl would be permitted:
This roughly corresponds to the
impl<T> BorrowFrom<Rc<T>> for T
example from the Little Orphan Impls blog post. As that blog post describes, this is an artifact of introducing ordering into the algorithm; the post describes various alternative rules, notably the "covered rule", that would permit both of these impls, but at the cost of other kinds of rules.Scenario. Wanting to implement a trait where the output type is not in current crate.
@Manishearth describes wanting to add an impl into libstd like so:
By rights, this impl should live in libcore, since all the input types are in libcore, but the output type is defined in libstd, so of course it cannot.
Scenario. Optionally derive traits if desired by others, but without adding a required dependency.
A common example is that my crate
foo
defines a typeFoo
which can be serialized (e.g., using the serde crate). However, I do not want to addserde
as a required dependency offoo
. Currently, best practice is to define aCargo.toml
feature (e.g.,with_serde
) that adds a dependency onserde
and includes the relevant impls. But this is a lot of manual work. Another example is how therayon
crate exports theParallelIterator
trait; other crates, likendarray
, might want to implement those, but without forcing a dependency onrayon
on othersProposals targeting this scenario:
This scenario was also described in #1553.
Other scenarios?
Please leave comments and we can migrate those descriptions into this header.
The text was updated successfully, but these errors were encountered: