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

is #[cfg] applicable to *some* non-stmt expressions? #32796

Closed
pnkfelix opened this issue Apr 7, 2016 · 17 comments
Closed

is #[cfg] applicable to *some* non-stmt expressions? #32796

pnkfelix opened this issue Apr 7, 2016 · 17 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Apr 7, 2016

Consider the following:

http://is.gd/28Q5H5

#![feature(stmt_expr_attributes)]

fn foo(_x: &[u32]) { }

fn main() {
    foo(&[1, #[cfg(not(now))] 2, 3])
}

Currently this is accepted (see #29850) but RFC 16 as written does not clearly say whether this is acceptable or not (see https://github.com/rust-lang/rfcs/blob/master/text/0016-more-attributes.md#cfg )

@pnkfelix pnkfelix added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Apr 7, 2016
@pnkfelix
Copy link
Member Author

pnkfelix commented Apr 7, 2016

cc @Kimundi

@pnkfelix
Copy link
Member Author

pnkfelix commented Apr 7, 2016

Lang team discussed this.

Opinions were mixed about whether this should be allowed or not.

But the more fundamental conclusion is that the decision here should go through the RFC process (merely as an amendment to RFC 16), rather than just having the lang team make a unilateral decision; we want to get input from the community about what is best here.

Note: It would be good to keep in mind that some use cases, such as function calls of the form

bar(1, #[cfg(no)] 2, 3)

are at the very least difficult to try to use, because one cannot write

fn bar(a: u32, #[cfg(no)] b: u32, c: u32) { ... }

@tbu-
Copy link
Contributor

tbu- commented Apr 8, 2016

I'm against such fine-grained cfg attributes. I think it'll produce unreadable code, similar to many #ifdefs inside the same term.

if(
#ifdef A
a
#endif
#if defined(A) && defined(B)
&&
#endif
#if defined(B)
b
#endif
#if defined(A) || defined(B)
&&
#endif
c)
{}

Compare that to

#[cfg(A)]      fn a() -> bool { a }
#[cfg(not(A))] fn a() -> bool { true }

#[cfg(B)]      fn b() -> bool { b }
#[cfg(not(B))] fn b() -> bool { true }

if a() && b() && c {
}

@pnkfelix
Copy link
Member Author

pnkfelix commented Apr 8, 2016

cc #15701 ... this is effectively an unresolved question that needs to be added to that tracking issue before we stabilize this feature.

@Kimundi
Copy link
Member

Kimundi commented Apr 10, 2016

So, obviously unreadable messes like what @tbu- showed should be considered unidiomatic in any case, but I can see the use cases where just plain item-level cfgs won't cut it.

One of the motivating examples that lead to me implementing RFC #16 was the use case of a huge struct with many #[cfg]-guarded fields, and the desire to be able to write a constructor function for it without having combinatorial explosion of the possible variations.

While what I implemented did not fully reach that state due to things like struct constructors like Foo { #[cfg] a: 5 } or as you said function argument lists not accepting attributes, it does allow for this:

#![feature(stmt_expr_attributes)]

struct Foo(
    u8,
    #[cfg(foo)] u16
);

fn main() {
    let x = Foo(1, #[cfg(foo)] 2);
}

As well as compile-time-cfg switching code like

let x = (#[cfg(a)] foo(), #[cfg(not(a))] bar()).0;

(Though I guess the latter might be replicable with blocks and assignment statements)

In my opinion, the fact that we already support attributes on struct fields and match arms might indicate that there is a desire to have them at more specific locations than just the "top level" items, types and statements, so maybe it would make sense to investigate whether it can be extend to places like patterns or argument lists so that things like #[cfg] can become more usable there.


Also, the way I see it all more complicated/unreadable constructions you can build with this would end up behind a macro anyway. For example , have a cfg-match macro:

macro_rules! cfg_match {
    (_ => $b:block) => {
        $b
    };
    ($cfg:meta => $b:block $($t:tt)*) => {
        (
            #[cfg($cfg)] $b,
            #[cfg(not($cfg))] (cfg_match!($($t)*)),
        ).0
    };
    () => {
        {
            panic!("Conditional code not supported under this configuration");
        }
    }
}

let x = cfg_match! {
    unix => { foo() }
    windows => { bar() }
};

@tbu-
Copy link
Contributor

tbu- commented Apr 11, 2016

That should be possible today, if I understand it correctly:

let x;
#[cfg(windows)]
{ x = 5; }
#[cfg(unix)]
{ x = 3; }

or even

#[cfg(windows)]
let x = 3;
#[cfg(unix)]
let x = 5;

@bluss
Copy link
Member

bluss commented Apr 14, 2016

@tbu- That needs the same feature (stmt_expr_attributes) to compile

@plietar
Copy link
Contributor

plietar commented May 4, 2016

There's a usecase for #[cfg] on expressions I find useful :

let backends = [
    #[cfg(feature = "foo-backend")] foo_backend,
    #[cfg(feature = "bar-backend")] bar_backend,
];

allowing multiple backends to be enabled at compile time using cargo features, and choosing one at runtime.
The alternative is to list all 2^N combinations, or use some macro magic to generate them (I've had to resort to this in order to stay on stable).

I guess you could consider this as a #[cfg] on a list element rather than an expression.

I'm more skeptical about other uses of it, especially on function calls.
if cfg!(...) { 3 } else { 5 } can be used for simpler cases (I assume this gets optimized out at compile time)

@brson
Copy link
Contributor

brson commented Jul 8, 2016

This looks like the only thing blocking stmt_expr_attributes from stabilizing.

@brson
Copy link
Contributor

brson commented Jul 8, 2016

Perhaps we can turn this behavior off and move forward with stmt_expr_attributes stabilization @rust-lang/lang? Readress this issue later.

@Kimundi
Copy link
Member

Kimundi commented Jul 9, 2016

Turn off in the sense of removal, or in the sense of adding a feature for it specifically?

@nrc
Copy link
Member

nrc commented Jul 10, 2016

I'm in favour of stabilising statement attributes and leaving expression ones under the existing feature flag (probably the easiest solution for users and the compiler, I'd also be happy with a new feature flag or removing expression attributes entirely, but either option seems less friendly).

@Mark-Simulacrum
Copy link
Member

So I'm not really clear what this issue is addressing. I think it's about attributes working on some expression and not others... but I don't see representative examples. Could someone summarize the current situation (@nrc?)?

@durka
Copy link
Contributor

durka commented May 6, 2017

Right now [1, #[cfg(unix)] 2, 3] is accepted on stable with no feature flag, so I'm not sure there is anything left to talk about here.

@nikomatsakis
Copy link
Contributor

It's so clearly useful... I think I'd be against trying to remove it. But I'd like to know the current status too.

@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 24, 2017
@pnkfelix
Copy link
Member Author

yes it appears that #36995 may have (accidentally?) stabilized it for list elements, which was the main case this issue was discussing... at this point I'm not sure we can afford to remove it...

@pnkfelix
Copy link
Member Author

pnkfelix commented Sep 22, 2018

So, as noted above, it appears that this (play):

fn foo(_x: &[u32]) { }

fn main() {
    foo(&[1, #[cfg(not(now))] 2, 3])
}

is now accepted on stable.

So regardless of whatever resoluton I was hoping to reach via discussion, it appears the matter has been resolved independently of the discussion here.

(Note to self: Maybe in the future when I discover oddities like this, I should immediately add compile-fail tests that are lacking the feature flag to ensure they don't get accidentally stabilized. At least, my interpretation of @nrc's stated position leads me to think that they did not intend to stabilize this aspect of attributes)

  • At least, this helps give me perspective on why some projects eagerly add test cases that are expected to fail (and even check them into the repository, tagged as expected failures) before a feature is implemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants