Skip to content

Move configuration 1 phase before crate metadata collection #25399

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 15, 2015

Conversation

lilyball
Copy link
Contributor

Stripping unconfigured items prior to collecting crate metadata means we
can say things like #![cfg_attr(foo, crate_type="lib")].

Fixes #25347.

@rust-highfive
Copy link
Contributor

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![cfg_attr(foo, crate_type="lib")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be included as a run-pass test instead? You could have an auxiliary file which uses conditional compilation to build a library and then also have a binary which uses conditional compilation to build itself as a binary instead (using // compile-flags to pass --cfg flags).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't quite sure how to make a run-pass test, but I've never actually looked at the auxiliary folder before so I wasn't really aware of how to do that. I'll try that now.

and then also have a binary which uses conditional compilation to build itself as a binary instead

What do you mean? Doesn't rustc assume it's a binary by default if not otherwise specified? I don't see how I can specify a file that would build as a library by default and override it to a binary with an attribute (since --crate-type on the command-line overrides #![crate_type]).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I think an auxiliary file won't actually work due to the compiler passing --crate-type, but this should work as a run-pass test because the compiler only assumes a binary output if no other is specified:

// src/test/run-pass/foo.rs

// compile-flags: --cfg foo
#![crate_type = "lib"]
#![cfg_attr(foo, crate_type = "bin")]

fn main() {}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked compiletest and it skips --crate-type if the auxiliary file specifies // no-prefer-dynamic (although I don't know why it doesn't just fall back to lib in that case). I could use the duplicate crate_type attributes like that but it results in a warning about the first one being unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, I suppose the warning is benign. It would be simpler to not have an auxiliary file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh huh, it's actually building both a binary and a library. I swear I tried this before and got a warning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, it doesn't work as a run-pass, because building two files like that doesn't seem to work with the -o flag (based on the error message, it seems to always result in the specified output path being the library).

@alexcrichton
Copy link
Member

Looks good to me, thanks! r=me with a switch to a run-pass instead of a run-make test.

Stripping unconfigured items prior to collecting crate metadata means we
can say things like `#![cfg_attr(foo, crate_type="lib")]`.

Fixes rust-lang#25347.
@lilyball lilyball force-pushed the crate-attributes-cfg_attr branch from 070d6b3 to 90b9529 Compare May 14, 2015 18:03
@lilyball
Copy link
Contributor Author

@bors r=alexcrichton

@bors
Copy link
Collaborator

bors commented May 14, 2015

📌 Commit 90b9529 has been approved by alexcrichton

bors added a commit that referenced this pull request May 15, 2015
…hton

Stripping unconfigured items prior to collecting crate metadata means we
can say things like `#![cfg_attr(foo, crate_type="lib")]`.

Fixes #25347.
@bors
Copy link
Collaborator

bors commented May 15, 2015

⌛ Testing commit 90b9529 with merge 2792855...

@bors
Copy link
Collaborator

bors commented May 15, 2015

@bors bors merged commit 90b9529 into rust-lang:master May 15, 2015
@sfackler
Copy link
Member

This isn't a total fix since some attributes like #[path] are processed directly in the parser.

@lilyball lilyball deleted the crate-attributes-cfg_attr branch May 15, 2015 05:10
@lilyball
Copy link
Contributor Author

@sfackler Good point, though #[path] presumably isn't usable at the crate level.

Which is to say, I can readily believe that #[cfg_attr(foo, path="bar.rs")] doesn't work, but the intention of this PR was to address crate-level attributes.

@klutzy
Copy link
Contributor

klutzy commented May 17, 2015

Filed #25544 for #[path] issue since it seems inherent to parser behavior.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#![cfg_attr] doesn't work for crate-level attributes
7 participants