Skip to content
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

rustc::traits::orphan_check_trait_ref does not consider TraitDef::is_marker #67919

Closed
Centril opened this issue Jan 6, 2020 · 4 comments
Closed
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. F-marker_trait_attr `#![feature(marker_trait_attr)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Centril
Copy link
Contributor

Centril commented Jan 6, 2020

It seems like #53693 did not update the orphan checker to consider #[marker] traits.

  • Suppose I have crates A, B, C.
  • A defines #[marker] trait Foo {}
  • B defines struct Bar;
  • C defines impl Foo for Bar {}

This should be OK because the overlap would actually be permitted as Foo is a #[marker] trait.

However, there might be something I've overlooked here such as impl polarity (impls_are_allowed_to_overlap) so I'm not filing a PR just now.

cc @nikomatsakis @arielb1 @scottmcm
cc #29864

References:

@Centril Centril added A-trait-system Area: Trait system T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Jan 6, 2020
@nikomatsakis
Copy link
Contributor

Hmm. This depends, I suppose, on the "role" of the orphan rules. Do they exist only to prevent overlap, or also to give a degree of "ownership" to the crate that defined Bar?

I suspect the answer should probably be the former, because the current rules don't actually give that much "ownership", especially for traits that have multiple input types. But I'm not entirely sure.

This same question arose in the context of the Pin unsoundness. In particular, as @withoutboats was telling me, they wanted to be able to reason about things like "there is no DerefMut impl for &T". I'm not crazy about this sort of "negative reasoning", as it is fragile -- it wen't wrong in the Pin case because of fundamental + dyn values. But in general if our orientation is to permit "third party crates" to add impls whenever we can get away with it, that is going to make such reasoning very difficult indeed.

@Centril
Copy link
Contributor Author

Centril commented Jan 6, 2020

I agree that orphan rules should be about preventing the potential for overlap.

In this specific case the user has marked the trait as #[marker] (and therefore it has no computational content and overlap is vacuously sound), so it seems to me that their intent was to allow this. Indeed, the reason I found this problem was that I wanted to move rustc::hir::ArenaAllocatable to rustc_arena, but couldn't because of the orphan rule.

As for DerefMut (which has computational content unlike a #[marker] trait), it seems to me that one can always retain such reasoning if one wishes by simply not using #[marker] for the specific trait. ;)

@Centril Centril added the F-marker_trait_attr `#![feature(marker_trait_attr)]` label Jan 9, 2020
@matthewjasper
Copy link
Contributor

Wouldn't this force us to consider that the following impl overlaps with anything?

impl <T: MarkerTrait> NonMarker for T {}

How is this supposed to interact with specialization?

@scottmcm
Copy link
Member

Closing this as @rust-lang/lang FCPed in #96766 (comment) that #[marker] is not about orphan. (It's only about overlap.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trait-system Area: Trait system C-bug Category: This is a bug. F-marker_trait_attr `#![feature(marker_trait_attr)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants