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
Closed

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

olsonjeffery opened this issue Apr 3, 2012 · 14 comments
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) C-enhancement Category: An issue proposing an enhancement or a PR with one.
Milestone

Comments

@olsonjeffery
Copy link
Contributor

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
Copy link
Contributor

brson commented Apr 3, 2012

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

@brson
Copy link
Contributor

brson commented Apr 3, 2012

#1724 is a related deficiency in cfg processing

@brson
Copy link
Contributor

brson 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
Copy link
Contributor

brson 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
Copy link
Contributor Author

sounds good to me!

@graydon
Copy link
Contributor

graydon 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 mentioned this issue Apr 5, 2012
@graydon
Copy link
Contributor

graydon 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
Copy link
Member

sanxiyn 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
Copy link
Member

sanxiyn 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
Copy link
Contributor

graydon 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
This adopts the syntax from #2119. No more annoying workarounds involving wrapping in mods!
@pcwalton
Copy link
Contributor

Fixed

@sanxiyn
Copy link
Member

sanxiyn commented Mar 21, 2013

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

@pnkfelix
Copy link
Member

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
Copy link
Member

closing as dupe of #17490

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Development

No branches or pull requests

7 participants