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

Change std inject attributes to outer attributes #14320

Closed
wants to merge 1 commit into from

Conversation

lilyball
Copy link
Contributor

The #[phase(syntax,link)] attribute on extern crate std needs to be an
outer attribute so it can pretty-print properly.

Also add #![no_std] and #[feature(phase)] so compiling the
pretty-printed source will work.

The #[phase(syntax,link)] attribute on `extern crate std` needs to be an
outer attribute so it can pretty-print properly.

Also add `#![no_std]` and `#[feature(phase)]` so compiling the
pretty-printed source will work.
@alexcrichton
Copy link
Member

Can tests be added for this? Injecting #![no_std] may not play nicely with macros that generate modules, perhaps we could leave it out for now?

@lilyball
Copy link
Contributor Author

@alexcrichton I don't know how to test this, but I ran make check-stage1-pretty and that passed. The only real testing I can think of is to use a run-make test and then look at the output of --pretty expanded for #![no_std], etc, but that doesn't really seem very useful.

Why would #![no_std] not play nicely with macros that generate modules? I would assume #![no_std] would not affect anything at all during this compilation (beyond where I already noted that it skips injecting the prelude based on #![no_std], which is why that attribute is deferred until the prelude inejct fold). Certainly a macro wouldn't be able to tell whether the crate is using #![no_std].

And when compiling the --pretty expanded output, #![no_std] is what prevents it from re-injecting the crates. The only reason today's --pretty expanded output compiles is because the #[phase(syntax,link)] attribute is skipped. Fixing it to print just that causes a link error due to duplicate crates.

@lilyball
Copy link
Contributor Author

FWIW I just searched for no_std and the only code that checks for its presence is rustc::front::std_inject. So I'm pretty confident in saying that adding it here won't affect anything else during this compilation.

@alexcrichton
Copy link
Member

macro_rules! foo( () => (mod foo { fn bar() -> Option<int> { Some(3) } }) )

foo!()

fn main() {
    foo::bar();
}

When expanded, this will have #![no_std] which will turn off prelude injection, which will cause mod foo to not compile in the expanded version (I think). Can you verify to see if this works?

Also, why is #![no_std] necessary for getting expanded pretty printing to compile?

@lilyball
Copy link
Contributor Author

@alexcrichton Still compiles fine (both the normal version and recompiling the --pretty expanded version). Macros are expanded before prelude injection happens (which should be obvious, otherwise the mod foo wouldn't get the prelude), and #![no_std] doesn't get added until prelude injection (otherwise it would disable prelude injection for the entire crate).

Also, why is #![no_std] necessary for getting expanded pretty printing to compile?

Because --pretty expanded spits out the stdlib-injected version, and skipping #![no_std] means the stdlib will get injected a second time. Apparently that's ok without the #[phase(syntax,link)] attribute, but with it, you get an error. So fixing the #[phase(syntax,link)] attribute to be an outer attribute instead of an inner one (which causes it to show up during pretty-printing) also breaks the recompilation of the expanded form without #![no_std].

You can reproduce this right now. The following compiles:

#![feature(phase)]
#![no_std]
#[phase(syntax,link)]
extern crate std = "std#0.11.0-pre";
extern crate native = "native#0.11.0-pre";
extern crate std = "std#0.11.0-pre";
extern crate native = "native#0.11.0-pre";
fn main() {}

This is basically what you get when you recompile the expanded form of fn main() {} right now (I skipped prelude injection because it's not relevant here, and of course I added #![no_std] so you don't inject a third copy of the stdlibs).

But the following, which is what you get after fixing the phase attribute, doesn't compile:

#![feature(phase)]
#![no_std]
#[phase(syntax,link)]
extern crate std = "std#0.11.0-pre";
extern crate native = "native#0.11.0-pre";
#[phase(syntax,link)]
extern crate std = "std#0.11.0-pre";
extern crate native = "native#0.11.0-pre";
fn main() {}

The error message says:

warning: using multiple versions of crate `std`
exp.rs:4:1: 4:37 note: used here
exp.rs:4 extern crate std = "std#0.11.0-pre";
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: crate_id: std#0.11.0-pre
exp.rs:7:1: 7:37 note: used here
exp.rs:7 extern crate std = "std#0.11.0-pre";
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
note: crate_id: std#0.11.0-pre
error: duplicate entry for `malloc`
error: duplicate entry for `free`
error: duplicate entry for `strdup_uniq`
error: duplicate entry for `eh_personality`
error: duplicate entry for `managed_heap`
error: duplicate entry for `gc`
error: aborting due to 6 previous errors

@alexcrichton
Copy link
Member

That error is quite worrisome (I opened an issue, #14330).

Also, it appears I need to go to bed sooner, of course the modules are expanded and have their preludes injected later!

bors added a commit that referenced this pull request May 21, 2014
…richton

The #[phase(syntax,link)] attribute on `extern crate std` needs to be an
outer attribute so it can pretty-print properly.

Also add `#![no_std]` and `#[feature(phase)]` so compiling the
pretty-printed source will work.
@bors bors closed this May 21, 2014
@lilyball lilyball deleted the fix_stdlib_inject_attrs branch May 21, 2014 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants