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 cfg_flags for boolean feature flags #321

Closed
wants to merge 2 commits into from
Closed

add cfg_flags for boolean feature flags #321

wants to merge 2 commits into from

Conversation

mobileink
Copy link

Add support for boolean config flags. --cfg 'feature="foo"' does not work; --cfg foo does. This change supports the latter.

Signed-off-by: Gregg Reynolds 601396+mobileink@users.noreply.github.com

mobileink added 2 commits May 28, 2020 09:17
Signed-off-by: Gregg Reynolds <601396+mobileink@users.noreply.github.com>
Signed-off-by: Gregg Reynolds <601396+mobileink@users.noreply.github.com>
@damienmg
Copy link
Collaborator

Thanks for the change, I was wondering if that needs an actual extra attribute. I haven't tested but it sounds like this could be done in rustc_flags, can't it? I could also make sense of going along as there is the crate_features next to it, but it feel kind of bloating the API to add more way to pass rustc flags.

@mobileink
Copy link
Author

I tried to use rustc_flags but could not make it work. Maybe I'm doing something wrong?
If I had my druthers I would rename crate_features to cfg_options. I think it makes sense to partition rustc options into cfg_options (key-val pairs), cfg_flags (booleans), and everything else, since the cfg options indirectly configure the source code, and the rest configure the compiler. That way the user interface makes a conceptual distinction explicit.

@damienmg
Copy link
Collaborator

damienmg commented Jun 2, 2020

Would you be willing to go ahead and adapt this PR to actually add cfg_options and cfg_flags and emit a warning when using the old options (noting that they are marked for deprecation, and how to switch to new options)?

This is a way larger change so I would understand if you rather not dig into that rabbit hole.

@mobileink
Copy link
Author

mobileink commented Jun 2, 2020 via email

@c0nk
Copy link

c0nk commented Jul 25, 2020

Would it make more sense to only add cfg_flags for both bools and key-values? This would make it similar to defines for C/C++ targets.

Base automatically changed from master to main January 28, 2021 20:47
@UebelAndre
Copy link
Collaborator

Any updates here? In Damien's absence, I wonder if @illicitonion might have some thoughts here?

@illicitonion
Copy link
Collaborator

Sorry, I'm struggling a bit to understand the use-case here that isn't met by the existing rustc_flags - would it be possible to add a test or example, in either the test or examples directory, showing some code which uses this feature, but which doesn't compile without it?

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

Successfully merging this pull request may close these issues.

6 participants