-
Notifications
You must be signed in to change notification settings - Fork 40
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
else can be used without if #23
Comments
I think this is just a mistake of the macro! It's not an intended use case but I don't really think it's that big of an issue to either document or prohibit. It seems like sort of a weird quirk at best? |
My gut reaction is that you shouldn't allow something like this because quirks like that that you aren't expecting could lead to bugs. This would be an opportunity to accidentally forget the I mean, imagine if If it is a big pain to fix from an implementation perspective, though, then I think that documenting the quirk would be reasonable enough. |
I agree with @alexcrichton. But I would add that I think an |
This is fixed on the current master, appears to be changed in 3499136 Any hope this gets released? The commit is one year old. I'd happily take this. I'm using |
I don’t think there’s no use case. I can imagine realistic code depending on this behaviour. Something along these lines: use cfg_if::cfg_if;
// Several different implementations of a thing
#[derive(Debug)] struct Windows;
#[derive(Debug)] struct Unix;
#[derive(Debug)] struct Fallback;
macro_rules! demo {
($($x:ident => $y:expr),*) => {{
cfg_if! {
$(
if #[cfg($x)] {
$y
}
)else*
else {
Fallback
}
}
}}
}
fn main() {
println!("{:?}", demo!());
println!("{:?}", demo!(unix => Unix, windows => Windows));
} That single-character change of (It’s pure coincidence that I’m commenting less than a day after you: I just happen to be starting doing detailed code review of crates starting with this one today.) |
Yes, this is technically a breaking change, so semver requires that release be 2.0.0. That's fine by me, I'd just like to see this released, it's frozen on master which is simply a waste :) |
@rust-lang/crate-maintainers Any thoughts on this? I think releasing it as 2.0.0 makes some sense but also concerned that would affect a lot of crates. The decision here seems important. |
I don't think this alone is worth a release, given that it would introduce a transition period where crates would be using both for a while. If we have other features motivating a major release then we could include this as well. But personally, I would love to see this included in the standard library instead. If people are going to go through a transition, let's have them go through a transition to the version in the standard library. |
That sounds great, thank you!
|
@JohnTitor There's already an internal copy in the stdlib; I put it there. The PR would just need to propose stabilizing it. (Also, you might add a comment next to the version number in |
If we're considering moving this to the standard library (which I wholly support!) then we should perhaps consider whether to introduce language-level syntax for this considering how widely used Regarding the issue itself, I agree that we should not do a 2.0.0 release just for this. |
Any progress on moving this to the standard library? |
TIL: https://lukaslueg.github.io/macro_railroad_wasm_demo
It clearly shows that an
else
can be used solely without a leading `if.A quick jump into the compiler shows it as well:
Is this intended? If yes, it should be documented, if not, it should be prohibited.
The text was updated successfully, but these errors were encountered: