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

Add covering set impl for generic version of trait when all values of a const are covered by impls #132

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JulianKnodt
Copy link

@JulianKnodt JulianKnodt commented Aug 3, 2023

When a trait contains a const: i.e. Foo<const C: bool>, if T : Foo<True>, T: Foo<False>, we should permit T : Foo<const C: bool> to be true. This can currently be emulated using specialization (altho how to do that I'm not sure), but this makes it unnecessary to use that feature to get this functionality.

I've currently only implemented it for bools (and I'm not sure if this implementation is sufficiently functional), but I'm not sure how generic it should be here

Is there a particular reason that an emoji is being used for the file extension?

p.s some changes are from running cargo fmt

@JulianKnodt JulianKnodt force-pushed the main branch 3 times, most recently from 0909bed to 403c98a Compare August 3, 2023 07:35
@oli-obk
Copy link
Contributor

oli-obk commented Jan 15, 2024

cross linking original Rust PR for easier discoverability: rust-lang/rust#104803

@BoxyUwU
Copy link
Member

BoxyUwU commented Jan 15, 2024

I've currently only implemented it for bools (and I'm not sure if this implementation is sufficiently functional), but I'm not sure how generic it should be here

if we have this feature it should work for arbitrary types

@oli-obk
Copy link
Contributor

oli-obk commented Jan 16, 2024

if we have this feature it should work for arbitrary types

well, at least for types that match exhaustiveness can handle ^^

@BoxyUwU
Copy link
Member

BoxyUwU commented Jan 16, 2024

all const generic types should have that property or else we've made a bit of a mess of the structuraleq thing i think haha

@JulianKnodt JulianKnodt force-pushed the main branch 8 times, most recently from 16e0ead to 65fe16a Compare January 29, 2024 09:47
@JulianKnodt JulianKnodt marked this pull request as ready for review January 29, 2024 09:47
@JulianKnodt
Copy link
Author

I would appreciate some guidance on how to implement such things (or at least comment on what I've implemented), as this repo has poor documentation and uses esoteric syntax and vocabulary

@JulianKnodt JulianKnodt force-pushed the main branch 2 times, most recently from 4b376dc to 4546c79 Compare January 30, 2024 08:32
@JulianKnodt
Copy link
Author

Who is actually reviewing this PR?
Is it @nikomatsakis?
How do I update tests so they do not use absolute paths in CI?

I would also note that there is no concept of structural equality or enums in this repo, so if you wanted me to add it for all kinds of ints, that would just be manually adding all of them.

Also adding in checks that fields of structs are all covered is also not something that is feasible in this repo since there are no existing tests with fields in structs.

If these points apply to the original PR, I'd appreciate if you could mark it there instead.

@apiraino
Copy link

I'll try assigning this review to those that provided feedback in #104803 (hope it helps to get some feedback). Please feel free to reassign, in case.

cc @lcnr

r? @oli-obk

@nikomatsakis
Copy link
Contributor

@JulianKnodt sorry I've let this sit so long! I can review it if you want to rebase, but I'd like to know a bit of context first. Is this something we've implemented in rustc already, or something you are proposing (for example)?

@oli-obk
Copy link
Contributor

oli-obk commented Jul 11, 2024

It was implemented in rustc, but never landed due to concerns about generalizing it to types other than booleans: rust-lang/rust#104803

@JulianKnodt
Copy link
Author

JulianKnodt commented Jul 11, 2024

If you'd like to review it that'd be great. I haven't looked at this or rustc for a while, but I still think that adding this would be good (even if my recollection is that the impl is a bit naive or limited in scope). It's currently a feature for if a struct with a generic const struct Example<const B: bool> has the implementations impl Trait for Example<true> and impl Trait for Example<false>, it then generically implements the trait: Example<const B>: Trait.

The bool case is the most common, next may be enums, and then int would be next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants