-
Notifications
You must be signed in to change notification settings - Fork 16
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
Switch away from enums towards strings in TargetInfo? #8
Comments
I would prefer to not use strings and constants, the available targets for rustc just do not change that often to justify it imo. That being said, having a fallback of other/unknown would be fine, since it could still be compared successfully as in your example by customizing the partialeq impls. Then there would be some work to infer the target pieces from a string, though that might be a little annoying as not all target specs have the same number of pieces, eg Darwin. |
Thanks for your response! So one use case that's popped up is being able to read a custom target specification and parse it into a platform: https://doc.rust-lang.org/rustc/targets/custom.html. These specifications may contain arbitrary data. Personally I'm not really a fan of the What do you think? |
Custom targets were something I thought about briefly when I did the initial implementation of this, but I didn't have a concrete example so punted that until I did. If you could give such an example that would be great! The enum approach works well for the regular targets built in to rustc which is what most users will care about, I would hate to lose them just because of more niche custom targets that only a few would. |
So my use-case is a custom OS I'm working on. I wanted to use In my libstd fork, I use
A lot of people working in embedded environment have to use custom targets. |
That's interesting, and does make sense to have for custom targets. We may even have some for ourselves for game consoles later (EmbarkStudios/rust-ecosystem#18). To me the |
I think that still has the problem that there are multiple ways to represent an enum and defining equality becomes very problematic. This is unfortunately becoming a really big issue. My suggestion still remains to model all of these parameters as strings and provide static strings for all the known targets. I believe it gets the best of both worlds. |
Using associated constants allows modeling an API that looks a lot like using an enum. pub struct Vendor<'a>(&'a str);
impl Vendor {
pub const pc: Vendor<'static> = Vendor("pc");
pub const unknown: Vendor<'static> = Vendor("unknown");
pub const uwp: Vendor<'static> = Vendor("uwp");
pub const nvidia: Vendor<'static> = Vendor("nvidia");
pub const sun: Vendor<'static> = Vendor("sun");
pub const fortanix: Vendor<'static> = Vendor("fortanix");
pub const wrs: Vendor<'static> = Vendor("wrs");
pub const rumprun: Vendor<'static> = Vendor("rumprun");
pub const apple: Vendor<'static> = Vendor("apple");
} This allows using it almost exactly like an enum: it can be used when pattern matching, creating an instance of the Vendor type, etc... match vendor {
Vendor::wrs => (),
Vendor::apple => (),
Vendor(other) => ()
} |
That could work nicely and easy to do since we auto-generate the builtin targets already. |
Yeah think that would work. Although it does look odd to have a static variable with lower case, and would generate a naming warning. So using standard SCREAMING_SNAKE_CASE would be cleaner. It does solve the equality issue that the |
Thank you so much! 😊 |
Is your feature request related to a problem? Please describe.
Target enums (e.g. https://docs.rs/cfg-expr/0.3.0/cfg_expr/targets/enum.Os.html) are currently modeled as a closed set. However, future versions of rustc may add new values to each of those. In addition, we have some use cases for custom platforms that aren't currently defined by rustc.
Describe the solution you'd like
TargetInfo
gains a lifetime parameter'a
.&'a str
instances, not enums.Describe alternatives you've considered
One option might be to add an
Other
variant:but that has consistency issues since e.g.
Os::haiku
isn't the same asOs::Other("haiku")
.I'd be happy to do the work if it sounds good to you!
Additional context
Thanks again for cfg-expr! We have several use cases for it, are already using it in target-spec, and would love to use it in tooling we're going to open source soon :)
The text was updated successfully, but these errors were encountered: