Allow attributes on match arms

[sfackler]

This is really only useful for #[cfg()]. For example:

enum Foo {
    Bar,
    Baz,
    #[cfg(blob)]
    Blob
}

fn match_foos(f: &Foo) {
    match *f {
        Bar => {}
        Baz => {}
        #[cfg(blob)]
        Blob => {}
    }
}

This is a kind of weird place to allow attributes, so it should probably
be discussed before merging.

[lilyball]

I didn't know we allowed attributes on enum variants. Is there anything else that's not an item that we allow attributes on already.

[sfackler]

They're also allowed on struct fields:

struct Foo {
   #[cfg(blah)]
   blah: int,
}

[lilyball]

Interesting. But both of those things are still structural components of an item. Whereas an arm of a match is a component of an expression.

It seems to me that if I can have an enum on a match arm, I should be able to have an enum on an expression itself (or maybe I should say on a statement). Right now we have the cfg!() macro to accomplish the same thing, although that unfortunately won't work for match arms.

I guess I'm saying that I'm not opposed to this change, it just feels a little weird, and I'd be more comfortable if we went ahead and allowed #[cfg(foo)] on statements as well.

[brson]

Is there a specific use case we're adding this for? Does it have workarounds?

[sfackler]

I have a use case here in rust-openssl: https://github.com/sfackler/rust-openssl/blob/master/ssl/mod.rs#L61

Many distributions of OpenSSL compile out SSLv2 support, so rust-openssl has the ability to do that as well. The enum entry can be simply tagged with a #[cfg(sslv2)] attribute, but functions that match on the enum have to be duplicated entirely.

[flaper87]

@sfackler you could have a static item that has the version of ssl and just duplicate the match:

if SSL_VERSION == 2 {
        match *self {
            Sslv2 => {return ffi::SSLv2_method();},
            _ => {}
        }
}

match *self {
        Sslv3 => {return ffi::SSLv3_method();},
        Tlsv1 => {return ffi::TLSv1_method();},
        Sslv23 => {return ffi::SSLv23_method();}
}

(obviously, not tested)

I'm not fully opposed to this change, though. I do agree with @kballard that it feels weird to have attributes for match arms and not for other things. Although, I'd prefer evaluate them in a case-by-case basis.

[sfackler]

@flaper87 that wouldn't compile if Sslv2 or ffi::SSLv2_method don't exist, though which wouldn't if SSLv2 is turned off. You'd also run into a noncomprehensive match error on the second match block if Sslv2 does exist.

[flaper87]

@sfackler I feel dumb today. 😢

You need the method to be duplicated in that case. So this change makes even more sense to avoid that much code duplication.

[eddyb]

I have another usecase (although LLVM might not have the annotation functionality required here):

fn main() {
    for _ in range(0, large_number) {...}
}
// Gets desugared to something like this:
// (also similar to what many iterator adapters do internally)
fn main() {
    match range(0, large_number) {
        ref mut iter => loop {
            match iter.next() {
                Some(_) => {...}
                #[cold] // now we can mark this arm "cold".
                None => break
            }
        }
    }
}

[sfackler]

How are people feeling about this? I'll close it if it seems too weird.

[lilyball]

It seems weird, certainly, but also useful. I'm inclined to say go ahead and do it, but I wish there was a better way to achieve the same result without the weirdness.

[andrew-d]

From someone just watching: I think it would be nice if this gets merged. It seems like it might be awkward to achieve matching on an enum in other ways while maintaining the nicety of having it warn you about an exhaustive match. Furthermore, from the examples above, even having two or three different combinations (e.g. SSLv2, TLS1.1 and TLS1.2 all being optional) means that implementing all the combinations individually seems impractical.

[flaper87]

This PR should probably have an RFC (see #12963). This change and the one proposing attributes on statements could probably be proposed on the same RFC.

[huonw]

RFC'd rust-lang/rfcs#16; any more commentary should move there.

[sfackler]

Reopened and rebased. r?

[alexcrichton]

Oh one thing I should mention is that we're trying to link commits to RFCs with just some words in the commit message, could you make sure this is at the bottom of the message:

RFC: 0008-match-arm-attributes

[sfackler]

I'll add that when I finish bludgeoning the pretty printer into submission :(