Allow incoherent trait implementations

[cvengler]

This commit adds two new attributes to rustc:

• #[rustc_has_incoherent_trait_impls]
• #[rustc_allow_incoherent_trait_impl]

They are intended to behave similar to the
#[rustc_has_incoherent_inherent_impls] pedants and serve the purpose to allow the implementation of incoherent trait implementations in the standard library.

According to the Rust reference, "a trait is considered incoherent if either the orphan rules check fails or there are overlapping implementation instances".

For now, this patch only circumvents orphan rule checks, hence why it is still a draft and because to my understanding, the problem of overlapping implementation instances is a larger one (see specializations).

Of course, circumventing the orphan rule is generally a bad idea, as it is to circumvent the coherence for inherent implementations. But just like the incoherence for inherent implementations, there are valid use-cases in the hacking with the standard library that would justify its use.

Personally, my motivation for this patch stems from rust-lang/libs-team#519, which describes a proposal for moving ToOwned from alloc to core and was theoretically accepted by libs team but ultimately dropped due to ToOwned implementations for types defined in core with associated types defined in alloc, thereby making it effectively impossible. I would like to keep continue working on this, but for this, a modification to rustc would probably be necessary.

[cvengler]

See also: cvengler@943fc34

For a demonstration of this in conjunction to moving ToBorrow to core.

[bjorn3]

the problem of overlapping implementation instances is a larger one (see specializations).

Arbitrary overlap of implementations where there is no clear specializing impl is not going to work. Which trait impl something gets resolved to needs to be consistent between crates. Otherwise you are likely to get an ICE due to type mismatches during codegen or const eval when typeck in one crate and codegen or const eval in another crate have a different view of which trait impl applies. I think only bypassing the orphan rule should ever be allowed.

Maybe not necessary in the initial implementation, but I've been thinking about having coherence domains. All crates within a coherence domain will apply the orphan rule as if they are the same crate. And once you have compiled all crates in a coherence domain you did let rustc check that there are no overlapping impls between the crates and produce a file as proof of this that you need to present when compiling dependent crates. With this you could have the entire standard library be in a single coherence domain and for example all crates of Rust for Linux be another coherence domain and similar for any other project that is developed as a single unit.

[BoxyUwU]

You will need T-types sign off before adding such an exception to the language even unstabley

[cvengler]

You will need T-types sign off before adding such an exception to the language even unstabley

Does this process involve opening an external ticket/RFC, similar to libs-team with their ACP system?

[cvengler]

the problem of overlapping implementation instances is a larger one (see specializations).

Arbitrary overlap of implementations where there is no clear specializing impl is not going to work. Which trait impl something gets resolved to needs to be consistent between crates. Otherwise you are likely to get an ICE due to type mismatches during codegen or const eval when typeck in one crate and codegen or const eval in another crate have a different view of which trait impl applies. I think only bypassing the orphan rule should ever be allowed.

That is what I would have thought too.
Not sure if the naming is a good idea then, as I went with how the Rust reference described it, alongside the allow_incoherent_inherent_impl naming schema.
Maybe a name mentioning the orphan rule explicitly would be better?

Maybe not necessary in the initial implementation, but I've been thinking about having coherence domains. All crates within a coherence domain will apply the orphan rule as if they are the same crate. And once you have compiled all crates in a coherence domain you did let rustc check that there are no overlapping impls between the crates and produce a file as proof of this that you need to present when compiling dependent crates. With this you could have the entire standard library be in a single coherence domain and for example all crates of Rust for Linux be another coherence domain and similar for any other project that is developed as a single unit.

I like the idea in theory but I feel it is a bit like a hypothetical pub(workspace) from which I have heard it is rather hard to realize.
Do you have concrete suggestion on how to use it?
The only idea I can come up would be something like #[rustc_coherence_domain("MAGIC-STR-IDENTIFIER")] though I am not sure if that is a good idea ...
In the end however, I think it would be a valuable addition, as breaking coherence is something you should only do if you control the entire supply chain above you, something that for sure applies to the standard library.

Regarding stability: To my understanding, all #[rustc_*] attributes are unstable as they are not part of the language but rather part of the compiler, though I could be wrong here too; unfortunately I could not find a quick resource on that online.

[fmease]

Regarding stability: To my understanding, all #[rustc_*] attributes are unstable as they are not part of the language but rather part of the compiler, though I could be wrong here too; unfortunately I could not find a quick resource on that online.

Yes, all attributes whose name starts with the string rustc (sic!) are automatically gated under the perma-unstable & internal feature rustc_attrs. They have no stability guarantees whatsoever and any misuse of the attribute by the user is not considered a compiler bug, even if it results in unsoundness in otherwise safe code.

Still, we don't want to add internal attributes willy-nilly since they would still need to be maintained.

[theemathas]

This sounds like it could maybe worsen the issue in #149347

[cvengler]

This sounds like it could maybe worsen the issue in #149347

This pull request does not touch any parts related to rustc_allow_incoherent_impl; its interface is just inspired by it.

[bjorn3]

It does have the same issue as rustc_allow_incoherent_impl though I think: Referencing a crate without actually using it can still cause method resolution to change due to the new trait impl being pulled in.

[cvengler]

Indeed, this would lead to a similar situation as in #149347 unfortunately. 🙁

I am still very new in contributing to the compiler but from what I understand so far, it is probably not the attributes "causing" the problem but rather the code trying to resolve the symbols which treats everything as it finds as being part of a global scope – basically what others have already mentioned.

Now the question is on how to proceed here.
On one side, I feel that fixing this is out of the scope of this pull request, on the other side I also feel that merging this would be a bit like pouring gasoline onto an already burning fire ...