resolve: Scale back hard-coded extern prelude additions on 2015 edition #54671

petrochenkov · 2018-09-29T14:13:56Z

#54404 stabilized feature(extern_prelude) on 2015 edition, including the hard-coded parts not passed with --extern.
First of all, I'd want to confirm that this is intended stabilization, rather than a part of the "extended beta" scheme that's going to be reverted before releasing stable.
(EDIT: to clarify - this is a question, I'm *asking* for confirmation, rather than give it.)

Second, on 2015 edition extern prelude is not so fundamentally tied to imports and is a mere convenience, so this PR scales them back to the uncontroversial subset.
The "uncontroversial subset" means that if libcore is injected it brings core into prelude, if libstd is injected it brings std and core into prelude.
On 2015 edition this can be implemented through the library prelude (rather than hard-coding in the compiler) right now, I'll do it in a follow-up PR.

UPDATE: The change is done for both 2015 and 2018 editions now as discussed below.

Closes #53166

petrochenkov · 2018-09-29T14:14:17Z

r? @nikomatsakis
cc @eddyb

eddyb · 2018-09-29T14:27:38Z

cc @rust-lang/lang

Centril · 2018-09-29T14:35:42Z

src/librustc_resolve/lib.rs

-        extern_prelude.insert(Symbol::intern("core"));
-        extern_prelude.insert(Symbol::intern("std"));
-        extern_prelude.insert(Symbol::intern("meta"));
+        if session.rust_2018() {


Why are we not doing the logic in 2015 in 2018 as well?

In 2018 crates outside of extern prelude are in disadvantage (they have to use crate::my_crate), so this would make life harder for #[no_std] people optionally using std under a feature (std-using paths work just fine in 2018 edition #![no_std] #53166 (comment)), unless something Add extern crate items to extern prelude #54658 -like is implemented.

For 2018 we still have enough time, while for 2015 extern prelude is already stabilized on beta.

I'd happily do this for 2018 as well despite 1. if it were urgent, since it's the most conservative variant.

Re. 2) -- the current beta becomes stable on 2018-10-26 so we should have time to wait with merging this until the next lang meeting (2018-10-04) right?

Re. 1) it is my understanding that given crate::my_crate, then the path segment my_crate will always refer to an item defined or used in the root of the current crate. Is that right?

It would be good if someone (not me) could write down all the rules for the module system in Rust 2015 and Rust 2018 somewhere so that it becomes a bit more easy to judge various changes; mainly it would be good since there have been so many small changes that it is hard reason about things.

As for people conditionally using std wouldn't they just write as @Nemo157 wrote in #54658?:

#![cfg_attr(not(feature = "std"), no_std)] use core::mem; #[cfg(feature = "std")] use std::boxed::Box;

Re. 2)

Sure.

Re. 1) it is my understanding that given crate::my_crate, then the path segment my_crate will always refer to an item defined or used in the root of the current crate. Is that right?

Yes.

As for people conditionally using std wouldn't they just write as @Nemo157 wrote in #54658?:

Yes, conditional #[no_std] would work if behavior on both edition is reset to what this PR does.
But perhaps it's impractical for some reasons? Need to ask people actually writing libraries.
As I understand, the motivation described in #53166 (comment) came from a lang-team meeting, so perhaps you remember something from that?

(As I said, I'd personally be totally okay with applying this PR to both edition.)

(P.S. I have to leave now and will be available only in couple of days.)

so perhaps you remember something from that?

Sorry, afraid not; but we'll know in a few days :)

Centril · 2018-09-29T14:35:45Z

src/librustc_resolve/lib.rs

+                if !attr::contains_name(&krate.attrs, "no_std") {
+                    extern_prelude.insert(Symbol::intern("std"));
+                    extern_prelude.insert(Symbol::intern("core"));
+                } else {


This seems like a weird way to write it?

extern_prelude.insert(Symbol::intern("core")); if !attr::contains_name(&krate.attrs, "no_std") { extern_prelude.insert(Symbol::intern("std")); }

Oh. Well, yes.
I've just reverted to the old version of code an it had that.
Will fix.

bors · 2018-10-01T00:59:36Z

☔ The latest upstream changes (presumably #54650) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2018-10-04T19:23:53Z

@petrochenkov what is this a relationship of this PR to #54658 ?

scottmcm · 2018-10-04T19:28:21Z

if libstd is injected it brings std and core into prelude.

I don't really want to encourage people to start using core all over the place, given the re-exports in std, all the examples that use std, the plan to flatten everything into std anyway, etc.

petrochenkov · 2018-10-05T09:30:59Z

@scottmcm
Seems like people still mostly prefer having core in prelude (#54658 (comment), #54658 (comment), #54658 (comment)) despite the "core encouragement" issue.
(I don't have much opinion on this personally.)

petrochenkov · 2018-10-05T09:39:55Z

@nikomatsakis

what is this a relationship of this PR to #54658 ?

This PR removes some names from prelude, #54658 gives a way to re-add them explicitly.

Generally, it could make sense to make this change after #54658 is done, but for 2015 edition

The issue is more urgent (due to feature(extern_prelude) stabilized on beta), and
The removal has much less impact due to "extern-prelude-ness" not being a crucial property in the old 2015 import system.

Centril · 2018-10-05T19:04:06Z

So I feel that we should do #54658 but with the change we agreed on in #54658 (comment), so the full behavior (end state) should I think be:

if edition_2015 =>
    extern_prelude += []

if edition_2018 =>
    #![no_core] =>
        extern_prelude += []

    #![no_std] =>
        extern_prelude += [core]

    default => // default means the absence of #![no_core] && #![no_std]
        extern_prelude += [core, std, meta]

    extern crate foo; =>
        extern_prelude += [foo]

    extern crate foo as bar; =>
        extern_prelude += [bar]

We only provide meta on default because we think it needs to depend on std...

Furthermore, due to usage of core and std as module names on 2015 crates, we can't add them to the extern prelude on 2015 unless #![no_std] is active.

...However, me and @nikomatsakis are not sure why we'd use the extern prelude set at all on 2015.
@petrochenkov do you have reasons why we'd use it on 2015?

petrochenkov · 2018-10-06T07:12:59Z

do you have reasons why we'd use it on 2015?

This is an interesting question - whether the stabilization of feature(extern_prelude) on 2015 was intended or not.
I actually asked it in the original post.

I think it would be better to enable extern prelude on 2015 edition.
Why:

First of all, we can do it. It's an extension rather than a change, extern prelude was active on 2015 edition since May (its actual use required a feature gate, of course) and we got no reported breakage / complaints.
So I don't understand what you are talking about in "due to usage of core and std as module names on 2015 crates, we can't add them to the extern prelude on 2015", preludes do not conflict with modules on 2015 edition. _{^{They won't conflict on 2018 either, eventually.}}
Second, it's better to minimize edition differences in general. Each behavior mismatch between editions will multiply issues in the future. Each behavior mismatch between editions also forces us to decide on the edition hygiene questions now. For example, what preludes are available to code coming from macros.
Finally, extern prelude actually makes code on 2015 very similar to 2018 model, which I also think is a great thing.
- Extern paths in non-imports can use crates directly (let x = my_crate::foo();).
- Extern paths in imports can use crates directly (use my_crate::foo;).
- Crate-local paths in both imports and non-imports can use crate::... by convention (let x = crate::bar();, use crate::bar;). That's exactly why crate::... was stabilized on 2015 edition as well despite not being strictly necessary - edition compatibility.
- But, you still need extern crate my_crate; in the root on 2015 for imports, that's pretty much the only difference.

petrochenkov · 2018-10-06T07:17:41Z

Minor point regarding meta - it's only there for future-proofing, and that future-proofing effect can only be observed with 2018 import rules, so it can be added only on 2018 as well. _{^{It's going to be removed from there as well, as soon as I finish the uniform path reimplementation.}}

nikomatsakis · 2018-10-08T21:41:23Z

@petrochenkov

So I don't understand what you are talking about in "due to usage of core and std as module names on 2015 crates, we can't add them to the extern prelude on 2015", preludes do not conflict with modules on 2015 edition

Hmm, perhaps I am confused. My impression was that we were enforcing a lack of ambiguity in order to retain forwards compatibility with universal paths -- so e.g. if there is a module std, and you have use std::foo::bar, that would be an ambiguity error. But I realize now that perhaps we only enforce that on Rust 2018, and that is considered distinct from the extern-prelude feature itself, which is rather more minimal, and simply adds crates into the prelude (thus achieving Java-style naming, for the most part). Does that sound correct?

Some testing :)

nikomatsakis · 2018-10-08T21:58:38Z

In that case, I guess we don't really need to alter the behavior between Rust 2018 and Rust 2015, right? We may wish to just exclude meta on 2015 altogether, though.

In which case it would be something like:

    #![no_core] =>
        extern_prelude += []

    #![no_std] =>
        extern_prelude += [core]

    default in 2015 => // default means the absence of #![no_core] && #![no_std]
        extern_prelude += [core, std]

    default in 2018 => // default means the absence of #![no_core] && #![no_std]
        extern_prelude += [core, std, meta]

    extern crate foo; =>
        extern_prelude += [foo]

    extern crate foo as bar; =>
        extern_prelude += [bar]

It seems like it's worth excluding meta from the 2015 default because it feels more "coherent" to me -- that is, in 2015, you get access to non-std crates via extern crate. It seems a bit random to have meta as an exception. (core is just a 'light' version of std, so that seems ok to me.)

petrochenkov · 2018-10-09T07:41:21Z

@nikomatsakis

Does that sound correct?

Everything looks correct in #54671 (comment).
Uniform paths are not going to happen on 2015 due to large-scale breakage, so we don't need to future-proof for them, and the module conflicts are a part of uniform path future-proofing rather than an inherent property of extern prelude.

petrochenkov · 2018-10-09T07:45:17Z

The scheme in #54671 (comment) is what I'd want to see implemented as well.

nikomatsakis · 2018-10-09T17:45:36Z

OK, I'm inclined to say that @petrochenkov if you update the PR to that scheme, I would r+. I think this reflects the rough consensus we arrived at in the @rust-lang/lang meeting (in particular, that #![no_std] ought to exclude std, etc).

petrochenkov · 2018-10-09T20:44:14Z

Argh, looks like recently merged #54650 broke everything.
Now I'll have to revert and reimplement it in such way that extern prelude is populated in some place where properties like no_std are already known.
(I'll probably do this tomorrow.)

sanmai-NL · 2018-10-11T13:42:53Z

@petrochenkov: though frustrating that change is for the better, isn’t it?

This reverts commit e90985a.

petrochenkov · 2018-10-13T19:35:30Z

@nikomatsakis
Updated.

nikomatsakis · 2018-10-16T15:56:54Z

@bors r+

bors · 2018-10-16T15:56:55Z

📌 Commit 894a8d5 has been approved by nikomatsakis

bors · 2018-10-17T01:59:34Z

⌛ Testing commit 894a8d5 with merge 37ba107...

resolve: Scale back hard-coded extern prelude additions on 2015 edition #54404 stabilized `feature(extern_prelude)` on 2015 edition, including the hard-coded parts not passed with `--extern`. First of all, I'd want to confirm that this is intended stabilization, rather than a part of the "extended beta" scheme that's going to be reverted before releasing stable. (EDIT: to clarify - this is a question, I'm \*asking\* for confirmation, rather than give it.) Second, on 2015 edition extern prelude is not so fundamentally tied to imports and is a mere convenience, so this PR scales them back to the uncontroversial subset. The "uncontroversial subset" means that if libcore is injected it brings `core` into prelude, if libstd is injected it brings `std` and `core` into prelude. On 2015 edition this can be implemented through the library prelude (rather than hard-coding in the compiler) right now, I'll do it in a follow-up PR. UPDATE: The change is done for both 2015 and 2018 editions now as discussed below. Closes #53166

bors · 2018-10-17T04:35:27Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 37ba107 to master...

pnkfelix · 2018-10-18T14:34:19Z

Discussed at compiler team meeting. beta-accepted.

@ghost

[beta] Rollup backports Merged and approved: * #54300: Updated RELEASES.md for 1.30.0 * #54939: rustdoc: don't prefer dynamic linking in doc tests * #54671: resolve: Scale back hard-coded extern prelude additions on 2015 edition * #55102: resolve: Do not skip extern prelude during speculative resolution r? @ghost

tikue · 2018-10-19T01:21:40Z

I think this might have broken futures-core-preview when the std feature is enabled (which it is by default).

error[E0433]: failed to resolve. Could not find `std` in `{{root}}`                                                                                                                                                                                                                       
   --> futures-core/src/future/future_obj.rs:194:9                                                                                                                                                                                                                                        
    |                                                                                                                                                                                                                                                                                     
194 |     use std::boxed::Box;                                                                                                                                                                                                                                                            
    |         ^^^ Could not find `std` in `{{root}}`

I bisected the problem to between bef62cc and 1dceadd.

futures-core is #[no_std] but when the std feature is enabled it includes certain items annotated with #[cfg(feature = "std")]. Does that sound like something that could have been broken by this?

cramertj · 2018-10-19T01:24:19Z

@tikue Yup, that's expected fallout. I'll issue a new release ASAP.

tikue · 2018-10-19T01:29:41Z

Cool, thanks @cramertj !

vhnatyk · 2019-03-17T22:34:51Z

hmm, I'm trying to do some embedded development on Cortex-M0 and well - I'm getting can't find crate for 'std' Turns out some dependencies of dependencies have extern crate std; And they are on crates.io - am I doomed? 😄 Or can I somehow still disable std globally. Reading through this issue as well as all related - doesn't make the picture more clear, unfortunately

petrochenkov · 2019-03-18T06:24:59Z

@vhnatyk
The Cortex-M0 problem is not directly related to this issue.
This issue is about "how to access standard library if it exists", while for the Cortex-M0 target it doesn't exist at all.
The only solution is to not use crates.io libraries that depend on std, or ask them to support #![no_std] mode.

vhnatyk · 2019-03-18T19:32:47Z

Seems like I still have to use Xargo (because of extern crate std; in dependencies) to have my own std, and I thought I don't have to because of no longer need Xargo... Then I though I can have extern crate std; somehow ignored globally 😯

rust-highfive assigned nikomatsakis Sep 29, 2018

petrochenkov added beta-nominated Nominated for backporting to the compiler in the beta channel. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 29, 2018

Centril reviewed Sep 29, 2018

View reviewed changes

Centril added I-nominated T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Sep 29, 2018

petrochenkov mentioned this pull request Oct 11, 2018

std-using paths work just fine in 2018 edition #![no_std] #53166

Closed

Centril removed the I-nominated label Oct 11, 2018

petrochenkov added 3 commits October 13, 2018 21:35

Revert "rustc_resolve: move extern_prelude from Resolver to Session."

83bb430

This reverts commit e90985a.

Copy extern prelude from resolver to global context

5229488

resolve: Scale back hard-coded extern prelude additions

894a8d5

petrochenkov force-pushed the extpre2015 branch from 80cf164 to 894a8d5 Compare October 13, 2018 19:34

nikomatsakis approved these changes Oct 16, 2018

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 16, 2018

bors mentioned this pull request Oct 17, 2018

resolve: Do not skip extern prelude during speculative resolution #55102

Merged

bors merged commit 894a8d5 into rust-lang:master Oct 17, 2018

bors mentioned this pull request Oct 17, 2018

Deprecate the FxHashMap() and FxHashSet() constructor function hack #55114

Merged

pietroalbini removed the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Oct 18, 2018

pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 18, 2018

pietroalbini mentioned this pull request Oct 18, 2018

[beta] Rollup backports #55184

Merged

pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 18, 2018

tikue mentioned this pull request Oct 19, 2018

futures-core-preview broken on latest nightly rust-lang/futures-rs#1296

Closed

ubnt-intrepid mentioned this pull request Oct 23, 2018

remove some impls of Value on no-std target slog-rs/slog#197

Merged

ExpHP mentioned this pull request Mar 23, 2019

0.3.0 release lloydmeta/frunk#154

Merged

petrochenkov deleted the extpre2015 branch June 5, 2019 16:21

resolve: Scale back hard-coded extern prelude additions on 2015 edition #54671

resolve: Scale back hard-coded extern prelude additions on 2015 edition #54671

Conversation

petrochenkov commented Sep 29, 2018 • edited Loading

petrochenkov commented Sep 29, 2018

eddyb commented Sep 29, 2018

Centril Sep 29, 2018

Choose a reason for hiding this comment

petrochenkov Sep 29, 2018 • edited Loading

Choose a reason for hiding this comment

petrochenkov Sep 29, 2018 • edited Loading

Choose a reason for hiding this comment

Centril Sep 29, 2018

Choose a reason for hiding this comment

petrochenkov Sep 30, 2018

Choose a reason for hiding this comment

Centril Oct 1, 2018

Choose a reason for hiding this comment

Centril Sep 29, 2018

Choose a reason for hiding this comment

petrochenkov Sep 29, 2018

Choose a reason for hiding this comment

bors commented Oct 1, 2018

nikomatsakis commented Oct 4, 2018

scottmcm commented Oct 4, 2018

petrochenkov commented Oct 5, 2018

petrochenkov commented Oct 5, 2018

Centril commented Oct 5, 2018

petrochenkov commented Oct 6, 2018

petrochenkov commented Oct 6, 2018

nikomatsakis commented Oct 8, 2018

nikomatsakis commented Oct 8, 2018

petrochenkov commented Oct 9, 2018

petrochenkov commented Oct 9, 2018

nikomatsakis commented Oct 9, 2018

petrochenkov commented Oct 9, 2018

sanmai-NL commented Oct 11, 2018

petrochenkov commented Oct 13, 2018

nikomatsakis commented Oct 16, 2018

bors commented Oct 16, 2018

bors commented Oct 17, 2018

bors commented Oct 17, 2018

pnkfelix commented Oct 18, 2018

tikue commented Oct 19, 2018

cramertj commented Oct 19, 2018

tikue commented Oct 19, 2018

vhnatyk commented Mar 17, 2019 • edited Loading

petrochenkov commented Mar 18, 2019

vhnatyk commented Mar 18, 2019

petrochenkov commented Sep 29, 2018 •

edited

Loading

petrochenkov Sep 29, 2018 •

edited

Loading

petrochenkov Sep 29, 2018 •

edited

Loading

vhnatyk commented Mar 17, 2019 •

edited

Loading