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

Refactor away the prelude injection fold #34108

Merged
merged 3 commits into from
Jun 9, 2016

Conversation

jseyfried
Copy link
Contributor

@jseyfried jseyfried commented Jun 6, 2016

Instead, just inject #[prelude_import] use [core|std]::prelude::v1::*; at the crate root while injecting extern crate [core|std]; and process #[no_implicit_prelude] attributes in resolve.

r? @nrc

@jseyfried
Copy link
Contributor Author

cc @petrochenkov @eddyb

@jseyfried jseyfried force-pushed the refactor_prelude_injection branch 2 times, most recently from a0cdaec to 1f01752 Compare June 6, 2016 10:23
@petrochenkov
Copy link
Contributor

This prevents using prelude in the same crate it is defined in, right? Something similar to #![local_prelude] from #32274. I wanted to use libcore's prelude in libcore and libstd's prelude in libstd after a snapshot.
I think, I'd still prefer #[prelude_import] to be an attribute on something mentioning the imported prelude module, like now, rather than on a whole crate.
I agree with the rest of the PR (the first commit breaks #34095 though).

@jseyfried
Copy link
Contributor Author

jseyfried commented Jun 6, 2016

EDIT: outdated comment

@petrochenkov

This prevents using prelude in the same crate it is defined in, right?

That isn't possible today (besides by using #![feature(prelude_import)] and manually adding #[prelude_import] use local::prelude::*; in each module, defeating the purpose of a prelude).

Something similar to #![local_prelude] from #32274. I wanted to use libcore's prelude in libcore and libstd's prelude in libstd after a snapshot.

Agreed, I want this too. I'm envisioning

#![feature(local_prelude)]

mod prelude {
    #[local_prelude]
    mod v1 { /* the libcore or libstd prelude */ }
}

or

#![feature(local_prelude)]
#![local_prelude]

mod prelude {
    mod v1 { /* the libcore or libstd prelude */ }
}

Either would very simple to implement after this PR.

I think, I'd still prefer #[prelude_import] to be an attribute on something mentioning the imported prelude module, like now, rather than on a whole crate.

If #[prelude_import] were intended for end users, I would agree. Since it is only for rustc-internal use, I don't think it matters.

the first commit breaks #34095 though

Yeah, the rebase straightforward though -- I'll PR against your branch if this happens to land first.

@petrochenkov
Copy link
Contributor

Either would very simple to implement after this PR.

Sure, I just expected custom and built-in, local and non-local preludes to work through the same mechanism without additional special attributes:

// Builtin non-local prelude from std
extern crate std; // <- injected automatically into the crate root
#[prelude_import] use std::prelude::v1::*; // <- injected automatically into the crate root

// Custom local prelude
#[prelude_import] use ::whatever::module::i::want; // <- written manually at the crate root, overwrites previous #[prelude_import] imports, module name is not hardcoded

Since it is only for rustc-internal use, I don't think it matters.

I'll use this argument the next time I come up with some horrible hack :)

@petrochenkov
Copy link
Contributor

Note, that with #[prelude_import] use path::*; GlobImport::is_prelude still can be refactored away, because prelude import is not really an import, only a marker, it can be ignored for all purposes except for supplying the prelude path.

@jseyfried jseyfried changed the title Refactor away prelude injection Refactor away the prelude injection fold Jun 7, 2016
@jseyfried
Copy link
Contributor Author

@petrochenkov good points, I updated this PR accordingly (see the edited initial comment).

I think it's simpler to keep GlobImport::is_prelude -- we'll need access to the prelude when expanding macros during import resolution (c.f. RFC 1560), and the prelude import might depend on other imports or not-yet-expanded modules, so we'll probably need to treat it like an ordinary import anyway.

@nrc
Copy link
Member

nrc commented Jun 7, 2016

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 7, 2016

📌 Commit 970e15d has been approved by nrc

Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 8, 2016
…n, r=nrc

Refactor away the prelude injection fold

Instead, just inject `#[prelude_import] use [core|std]::prelude::v1::*;` at the crate root while injecting `extern crate [core|std];` and process `#[no_implicit_prelude]` attributes in `resolve`.

r? @nrc
@bors
Copy link
Contributor

bors commented Jun 9, 2016

⌛ Testing commit 970e15d with merge 24526cc...

bors added a commit that referenced this pull request Jun 9, 2016
Refactor away the prelude injection fold

Instead, just inject `#[prelude_import] use [core|std]::prelude::v1::*;` at the crate root while injecting `extern crate [core|std];` and process `#[no_implicit_prelude]` attributes in `resolve`.

r? @nrc
@bors bors merged commit 970e15d into rust-lang:master Jun 9, 2016
@jseyfried jseyfried deleted the refactor_prelude_injection branch February 12, 2017 13:06
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.

4 participants