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

support overlap for marker traits #62

Closed
nikomatsakis opened this Issue Oct 22, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@nikomatsakis
Copy link
Collaborator

nikomatsakis commented Oct 22, 2017

The current coherence rules do not have any special cases for "marker traits" -- that is, traits with zero items. As discussed in #8, at least for now it'd be nice to have "marker-ness" be declared explicitly with #[marker].

Here are some steps to do this:

We want to model this on the support for #[auto] trait declarations. This means we would:

It might be nice, rather than just adding another boolean field, to introduce some sort of TraitFlags type (e.g., struct TraitFlags { auto: bool, marker: bool }) into the AST and copy that over into ir::TraitDatumBound? Or at least I don't love having a bunch of random boolean variables floating around.

@nikomatsakis

This comment has been minimized.

Copy link
Collaborator Author

nikomatsakis commented Oct 22, 2017

cc @mikeyhew

cc @withoutboats -- do you think further changes are needd in coherence?

@lqd

This comment has been minimized.

Copy link
Contributor

lqd commented Oct 22, 2017

I'd like to try this if that's ok :)

@nikomatsakis to add the keyword to the parser, should we ensure #[auto] and #[marker] can or can't be used at the same time ? (maybe it doesn't matter).

Should we also add the #[marker] attribute to the other marker traits in the solver tests ? (the ones named Marker specifically, since most other tests are marker traits as well ?)

btw I quickly prototyped the steps in the issue, and indeed nothing more seems to be needed to solve the problem.

EDIT: In the meantime I've opened #64 as WIP with those questions

@mikeyhew

This comment has been minimized.

Copy link
Contributor

mikeyhew commented Oct 22, 2017

@lqd I was going to take this on, but I'm already busy with an issue from rust-lang/rust, and you seem to have a good idea of what needs to be done here, so go for it :) When I'm finished with the compiler issue, I'm sure @nikomatsakis will have more issues to work on for chalk.

#[auto] and #[marker] can both be used at the same time – think of Send and Sync. As for adding #[marker] to traits in the existing tests, that sounds like a good idea, but I'll let @nikomatsakis or @withoutboats direct you there. There is one test case though that I added in #61, partial_overlap_3, which currently doesn't pass, and should pass once you implement this and add a #[marker] attribute to the Marker trait.

@lqd

This comment has been minimized.

Copy link
Contributor

lqd commented Oct 22, 2017

@mikeyhew thanks for the comment ! As you mentioned, partial_overlap_3 indeed passes successfully with #64

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.