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

Prohibit multiple predicates #37

Merged
merged 1 commit into from Oct 6, 2020
Merged

Prohibit multiple predicates #37

merged 1 commit into from Oct 6, 2020

Conversation

ghost
Copy link

@ghost ghost commented Oct 2, 2020

I noticed that cfg-if uses $($meta:meta),* to match cfg predicates:
https://github.com/alexcrichton/cfg-if/blob/f71bf60f212312faddee7da525fcf47daac66499/src/lib.rs#L37
https://github.com/alexcrichton/cfg-if/blob/f71bf60f212312faddee7da525fcf47daac66499/src/lib.rs#L51
https://github.com/alexcrichton/cfg-if/blob/f71bf60f212312faddee7da525fcf47daac66499/src/lib.rs#L53

That is redundant: multiple cfg predicates is not allowed by rustc, and specifying multiple predicates results in #[cfg]s that do not make sense in else parts. Also, mistakes can be easily made when there're many parentheses and commas in the cfg predicate.

@ghost ghost changed the title Prohibited multiple predicates Prohibit multiple predicates Oct 2, 2020
@ghost ghost closed this Oct 2, 2020
@ghost ghost deleted the prohibited-multiple-predicates branch October 2, 2020 19:26
@ghost ghost restored the prohibited-multiple-predicates branch October 2, 2020 19:26
@ghost
Copy link
Author

ghost commented Oct 2, 2020

Sorry about the typo, but I can't rename the branch.

@ghost ghost reopened this Oct 2, 2020
@alexcrichton
Copy link
Member

Oops I think I was a bit too zealous when I first wrote this! Can you gist what the error message for multiple predicates looks like before and after?

@ghost
Copy link
Author

ghost commented Oct 6, 2020

Before the patch, specifying multiple predicates compiles, but for cfg_if! { if #[cfg(A, B)] {} else {} }, the if part is guarded by #[cfg(all(A, B))], the else part is guarded by #[cfg(not(any(A, B)))] (instead of #[cfg(not(all(A, B)))]).

After the patch, the compilation fails with error: no rules expected the token `,`.

Note: The "native" #[cfg(A, B)] fails with error: multiple `cfg` predicates are specified.

@alexcrichton
Copy link
Member

Ah ok so it does compile but it's actually quite confusing how it works. Definitely unintended, thanks for catching this!

@alexcrichton alexcrichton merged commit 7daa598 into rust-lang:master Oct 6, 2020
g2p added a commit to g2p/bolero that referenced this pull request Nov 20, 2020
This only contains a trivial change:
<rust-lang/cfg-if#37>
camshaft pushed a commit to camshaft/bolero that referenced this pull request Nov 20, 2020
This only contains a trivial change:
<rust-lang/cfg-if#37>
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.

None yet

1 participant