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

cfg() and multiple predicates, logic language, etc #2119

Closed
olsonjeffery opened this issue Apr 3, 2012 · 14 comments

Comments

Projects
None yet
7 participants
@olsonjeffery
Copy link
Contributor

commented Apr 3, 2012

Quick over: the cfg() attribute can't be used to do multiple-declaration of rust types/fns/mods along multiple-dimensions of consideration (OS, arch) without signifigant hurdle-jumping-through'ing. I spoke with @graydon about this, briefly, on IRC and he was under the impression that commas in a list passed to cfg() would behave like an &&, but it seems, from my observation, that they act more like ||.

Here's a test case:

use std;
import io;

fn main() {
  io::println("#[cfg()] failure test case");
  let inst = new_foo_inst();
}

#[cfg(target_os="linux", target_arch="x86_64")]
type foo = {
  a: int, b: int
};
#[cfg(target_os="linux", target_arch="x86")]
type foo = {
  a: int, b: int, c: int
};

fn new_foo_inst() -> foo {
  ret new_foo_arch();
  #[cfg(target_arch="x86_64")]
  fn new_foo_arch() -> foo {
    ret { a: 0, b: 0 };
  }
  #[cfg(target_arch="x86")]
  fn new_foo_arch() -> foo {
    ret { a: 0, b: 0, c: 0};
  }
}

Will produce this compiler output:

./cfg.rs:10:0: 12:2 error: duplicate definition of type foo
./cfg.rs:10 type foo = {
./cfg.rs:11   a: int, b: int
./cfg.rs:12 };

There are workarounds, of course (As my new_foo_inst() impl shows, can acheive similar with nested branches of os::arch modules). But I think we can do better.

I know this isn't high-priority (as I mentioned, I've already worked around the problem... and I'm not after a turing-completeness for cfg() :) , but it would propose changes the cfg() impl to:

  1. Change the , in cfg() to be ||.
  2. Add && as a supported separator, with all that it implies
  3. Support arbitrary, if-like predicate nesting within cfg() using parenthesis

Also, interesting that a single = is used for "equals", here. Trying hard to avoid the impulse to not be pedantic about consistency across the language (== is used for such things in the main grammar).

This is totally outside the level of my experience with the language internals so far, and I gather would involve syntax changes. I'm interested in getting my feet wet, gently, with this. Assuming that such a change would be accepted, where would I (or anyone else interested in this work) start?

@brson

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2012

Somewhat related to #1242 and the ongoing discussion about improvements to attributes.

@brson

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2012

#1724 is a related deficiency in cfg processing

@brson

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2012

Agreed that conditional compilation needs to be at least slightly more expressive. We do have to keep in mind that cfg specs are written the general attribute grammar, so adding things like operators needs to make sense for attributes in general.

I want to help you get started on this. Let's come up with something minimal that solves some real issues in the existing code base and that modifies the attribute grammar in sensible ways, and keeping in mind the issues and designs discussed in #1242. I know that @graydon and dylukes had some long discussions about ways to overhaul attributes.

Regarding = in attributes, I think it might make sense to switch to : to match records. If we could make attributes have the same structure as some subset of rust types that could be really useful.

@brson

This comment has been minimized.

Copy link
Contributor

commented Apr 3, 2012

What if we make cfg also accept cfg(and(...)), cfg(not(...)) and cfg(or(...)) and switch the default behavior to 'and' instead of 'or'?

@olsonjeffery

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2012

sounds good to me!

@graydon

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2012

Glad this topic is reviving a bit. Dylukes and I did discuss this but didn't really finish the conversation, I'm happy to see someone else run with it too. It needs doing. What we boiled things down to was a general sense that:

  • A slightly richer set of datatypes and expression language in attributes would be useful, though I argued that outside-in evaluation (normal order) probably makes more sense for attributes, as they function more like cpp or macros than like program code. Possibly include relational operators < <= == != and boolean operators || && ! and such.
  • A more open-ended set of "functions" that operate on (and return) attribute-values should probably do most of the real lifting, punting to compiler-provided extensions (as with syntax extensions etc.). The current attribute nodes like test and cfg can then operate as leaf functions that cause side effects on their attached item and return the empty attribute
  • Among new functions, some that can set variables and read them back out of the configuration dictionary would be useful, probably with variable scope following the module tree (top-down)

We didn't implement any of this, but I'd be happy to see any work along these lines.

@graydon graydon referenced this issue Apr 5, 2012

Closed

#[cfg_attr] #1242

@graydon

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2012

Also see https://github.com/mozilla/rust/wiki/Proposal-for-attribute-preprocessing though personally I think it's doing too much and we should be motivated by the actual cases we're running into, which are mostly low-complexity combinations of boolean logic and variable-like reuse of a given test.

@sanxiyn

This comment has been minimized.

Copy link
Member

commented Jan 23, 2013

It seems that this was discussed in https://github.com/mozilla/rust/wiki/Meeting-weekly-2013-01-15 but I am not clear which syntax was agreed upon. Can you give an example?

@sanxiyn

This comment has been minimized.

Copy link
Member

commented Feb 22, 2013

From IRC: syntax agreed on was to encode a or (b and (not c)) as

#[cfg(a)]
#[cfg(b, not(c))]

I will try to implement this.

@graydon

This comment has been minimized.

Copy link
Contributor

commented Mar 19, 2013

We discussed a little more in the meeting today and concluded that in the longer-term we'd prefer to solve this by:

  • Adopting an actual expression grammar including and, or and not nodes
  • Prohibiting multiple cfg attributes on the same item

The idea being that neither juxtaposed-cfg-attributes nor terms-in-a-comma-list clearly imply AND or OR behavior, and that in any case forcing users to write config conditions in conjunctive or disjunctive normal form is potentially painful.

In the short term, and in a backward-compatible sense, we will land pull req #5410

bors added a commit that referenced this issue Mar 20, 2013

auto merge of #5410 : luqmana/rust/cfg-and, r=graydon
This adopts the syntax from #2119. No more annoying workarounds involving wrapping in mods!
@pcwalton

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2013

Fixed

@pcwalton pcwalton closed this Mar 20, 2013

@sanxiyn

This comment has been minimized.

Copy link
Member

commented Mar 21, 2013

@graydon asked this issue "to remain open for now" in #5410...

@pnkfelix

This comment has been minimized.

Copy link
Member

commented Nov 18, 2013

We should revisit graydon's note from 8 months ago: it says we should disallow multiple cfg on a single item. If that is still the plan, it needs to be implemented.

Otherwise, the current semantics (that multiple cfg attributes compose as OR, not as AND; and also that and does not exist, and instead is implied by feeding in multiple arguments to a single cfg) needs to be documented somewhere.

@pnkfelix

This comment has been minimized.

Copy link
Member

commented Sep 25, 2014

closing as dupe of #17490

@pnkfelix pnkfelix closed this Sep 25, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.