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

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

Closed
cramertj opened this issue Aug 7, 2018 · 36 comments
Closed

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

cramertj opened this issue Aug 7, 2018 · 36 comments
Assignees
Labels
P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@cramertj
Copy link
Member

cramertj commented Aug 7, 2018

Example:

#![no_std]

use std::io::{Write, stdout};

fn main() {
    stdout().write(b"oh dear...\n").unwrap();
}

rustc happily accepts this when passed --edition 2018. This makes sense-- it was always possible to reach std in #![no_std] crates if you wrote extern crate std;, but now that extern crate is redundant, that is no longer necessary. Unfortunately, that also means that projects attempting to test for #![no_std] compatibility no longer have an easy way to check that they've correctly cut out all std-based dependencies.

We could special case std paths to not work via the "extern prelude". This would work, but would require projects that want to optionally use std (who currently use #[cfg(feature = "std")] extern crate std;) to continue using extern crate std;, which is awkward both because (1) it'd be the only place where extern crate ever gets used in the Rust of the future and (2) you'd have to write paths like crate::std::... in order to refer to your locally-mounted extern crate std, which is all sorts of strange.

Another option could be to make cargo and rustc aware of "no-std-ness" and allow some sort of flag or option which could make std un-linkable, perhaps usable by something like this Cargo.toml:

[dependencies]
std = { optional = true }

[features]
default = ["std"]

However, this seems like a fairly complex addition and begins to interact with the various std-aware-cargo proposals.

Another option: do nothing. This isn't a huge deal, and as @MajorBreakfast pointed out, the issue will often be caught in CI by testing against targets that don't have std support. There are also various proposals like the portability lint that could obsolete whatever change we come up with here.

@joshtriplett
Copy link
Member

I'd certainly like to see std handled through cargo someday. In the meantime, how difficult would it be to have #![no_std] make std disappear from the prelude?

@petrochenkov
Copy link
Contributor

petrochenkov commented Aug 7, 2018

std is not in the prelude when #![no_std] is enabled.

In the "anchored path" model (using terminology from #53130) imports still desugar into absolute paths (::std::io::... aka extern::std::io::...) and such paths can refer to any crates that can be found in crate search directories, not only those from the extern prelude.

@petrochenkov
Copy link
Contributor

In the "uniform path" model std cannot be accessed via relative paths in no-std crates (since it's not in the prelude), but still can be accessed with explicit absolute paths:

#![no_std]

use std::io; // ERROR
use ::std::io; // or `use extern::std::io;`, OK

@joshtriplett
Copy link
Member

such paths can refer to any crates that can be found in crate search directories, not only those from the extern prelude.

Then let me rephrase my original suggestion: how difficult would it be to have #![no_std] remove std from whatever list of crates that :: searches? (And are there any crates other than std that this applies to?)

@petrochenkov
Copy link
Contributor

Not difficult at all, just weird.
A lint would probably be more appropriate for this situation.

@cramertj
Copy link
Member Author

cramertj commented Aug 8, 2018

@joshtriplett I also suggested this and discussed the tradeoffs in my post above:

We could special case std paths to not work via the "extern prelude". This would work, but would require projects that want to optionally use std (who currently use #[cfg(feature = "std")] extern crate std;) to continue using extern crate std;, which is awkward both because (1) it'd be the only place where extern crate ever gets used in the Rust of the future and (2) you'd have to write paths like crate::std::... in order to refer to your locally-mounted extern crate std, which is all sorts of strange.

I don't think it's a slam-dunk, and I agree with @petrochenkov that a warning is probably a good solution in this case, since crates could conditionally deny the warning or allow it based on their std feature gate.

@nikomatsakis
Copy link
Contributor

@petrochenkov so, just to be clear:

I think that right now if you do use some_crate::bar, we will go and search the lib path for some_crate, just as we used to with extern crate. This presumably is why std works, right?

I remember finding that somewhat surprising to start. I expected us only to use the crate names that were given explicitly to rustc via --extern for such paths, and to require explicit extern crate in order to actually search the disk.

I guess though that if we were to try and change it, we would have to make sure we handle core, std, and friends correctly here.

@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 5, 2018

@nikomatsakis

I think that right now if you do use some_crate::bar, we will go and search the lib path for some_crate, just as we used to with extern crate.

Yes, right now with 2018 edition enabled use path; will go and search library directories (unless uniform_paths is enabled) and ::path will always go and search library directories.

These discussions are scattered over several threads, and in one of them a few people, including @eddyb , expressed the desire to disable this directory search, or use it only for crates explicitly passed with --extern without path argument.

@aturon aturon added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Sep 6, 2018
@aturon
Copy link
Member

aturon commented Sep 6, 2018

We discussed this issue in the lang team meeting today, and arrived at a rough consensus which I'll try to lay out here. cc @rust-lang/lang

  • We do not search library directories in Rust 2018, except for the three currently-stable libraries (core, std, proc_macro) which are whitelisted.

  • We provide a lint against using std, which is normally allow by default, but is deny by default in no-std cases. Crate authors providing their own std flag can conditionally allow the lint based on this flag.

  • We will provide an unstable mechanism to specify additional root crates in Cargo.toml, which will be passed in via --extern, so that you can get access to e.g. rustc crates.

    • In the long run, this mechanism will provide a way to stabilize new root crates and provide access to them.

@SimonSapin
Copy link
Contributor

We provide a lint against using std, which is normally allow by default, but is deny by default in no-std cases.

In this new world, how does one achieve the existing pattern that goes like this? #![no_std] #![cfg(feature = "std")] extern crate std;

@joshtriplett
Copy link
Member

@SimonSapin #![cfg_attr(not(feature="std"), no_std)] might help.

@Nemo157
Copy link
Member

Nemo157 commented Sep 7, 2018

@joshtriplett the problem with that is you then also need to have

#[cfg(feature = "std")]
use std::mem;
#[(cfg(not(feature = "std"))]
use core::mem;

everywhere (or some similar sort of pattern).

I think that the direct replacement for @SimonSapin’s pattern is

#![no_std]
#![cfg_attr(feature = "std", allow(the_lint_aturon_mentions))]

Edit: hopefully the long term plan will allow abandoning this and having std explicitly added to the prelude based on features via the mechanism in Cargo.toml.

Edit2: actually, testing 2018 in the playground it allows access to core even without no_std, so @joshtriplett’s attribute would actually work (depending on what the lint lints against).

@eddyb
Copy link
Member

eddyb commented Sep 11, 2018

Re-nominating because @withoutboats brought up an interesting point:

  • Do we really want to keep proc_macro as a name?

While @withoutboats would prefer to use a less specific/jargon-y name, potentially an umbrella term for future compilation-related facilities, my interest in this has to do with the whitelist, so specifically:

  • Do we really want to have proc_macro on equal footing with core and std, forever?

I was already feeling uneasy about proc_macro being far too specific to belong in the whitelist, and how the only reason we're doing this is to avoid needing extern crate / --extern / Cargo opt-in.

@lqd mentioned "metaprogramming", from which one could derive meta, as a crate name.
I think it's an extremely good umbrella term because of how short and nonspecific it is, while at the same time having a clearly delineated purpose (Rust metaprogramming, of all sorts).

IIUC, the current migration infrastructure can be easily adjusted to suggest replacing extern crate proc_macro; ... use proc_macro::TokenStream; with use meta::proc_macro::TokenStream; (or even use meta::tokens::TokenStream;).

To be clear, I don't mind keeping "proc macro" jargon around (e.g. the crate type and attributes), but having a crate (other than core and std) available without any opt-in, requires more consideration.

cc @alexcrichton @dtolnay

@alexcrichton
Copy link
Member

Sorry I haven't been following this thread and the last comment seems not entirely related to the OP, but for proc_macro specifically I feel like the ship has sailed for the 2018 edition, we're way too close to the cutoff for new features (in theory 2 days) to consider something like dropping proc_macro as a name.

@petrochenkov
Copy link
Contributor

Do we really want to have proc_macro on equal footing with core and std, forever?

I think we can keep proc_macro outside of extern prelude until --extern without path and its support in Cargo are stabilized. It may take 1-2-3 releases I guess? At least in rustc the feature is almost trivial.

Proc macros is an advanced feature so beginners learning Rust from 2018 edition will unlikely hit the "you don't need extern crate except for this one case" problem, and proc macro authors can certainly wait for a couple of months longer with removal of extern crate from code.

@eddyb
Copy link
Member

eddyb commented Sep 11, 2018

@alexcrichton I don't want to get rid of proc_macro as a name, but rather "hide it somewhere".
(as opposed to everyone being able to use proc_macro::...; in any crate)

If we keep extern crate, it seems okay to require it for proc_macro, although there was some speculation that we might be able to remove/unstabilize the extern crate syntax in the edition.

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Sep 13, 2018

We discussed this in the @rust-lang/lang meeting. I forget who was supposed to write up the notes. =) I believe the general feeling was that:

  • meta is a better name
  • but we're not sure of the details of how said meta crate should be laid out
  • it's late to change proc_macro etc

Therefore, what we could do is to (a) reserve the name meta but not map it to anything in particular, so we can add it later and (b) continue to require proc_macro implementations to write extern crate for the time being.

Personally, I think it would also be ok to permit proc_macro absolute paths and just deprecate that name at some later point (when we introduce the meta crate) but I don't have a strong opinion about it.

Thoughts?

@Centril
Copy link
Contributor

Centril commented Sep 13, 2018

I forget who was supposed to write up the notes. =)

eddyb 😛

Therefore, what we could do is to (a) reserve the name meta but not map it to anything in particular, so we can add it later and (b) continue to require proc_macro implementations to write extern crate for the time being.

I think it's a 👍 idea.

Personally, I think it would also be ok to permit proc_macro absolute paths

Not super excited about use proc_macro::foobar; on 2018.

@sanmai-NL
Copy link

sanmai-NL commented Sep 14, 2018

@nikomatsakis: as an outsider who prefers robustness/stability, I still think meta or better metaprogramming is much better and that breakage from the name change should be okay. One advantage is friendliness in a context of programming education in Rust. You want the learner to be reminded of the fundamental meaning of things, not of implementation details.

What is the interaction between this change and the use of proc_macro2?

@eddyb
Copy link
Member

eddyb commented Sep 14, 2018

@sanmai-NL We're unsure yet what we'll do to proc_macro exactly, but everything that works today will continue to work on Rust 2015, and if Rust 2018 requires you to go through meta, it will do automatic migrations for imports.

The decision we took is mainly to reserve meta as "available without Cargo opt-in or extern crate", which is something we're only doing in Rust 2018 to core, std and meta.

We might even want an RFC or at least a FCP'd PR adding a meta crate, and debate its structure there (we can reexport proc_macro under it, but we're not sure we want to do that, or something nicer).

As for proc_macro2, any code migrating to Rust 2018 shouldn't need it anymore, since Rust 2018 implies the proc_macro2 APIs are stable under proc_macro.
And if we make the proc_macro APIs accessible through meta, all code on Rust 2018 will end up going through meta, regardless of whether it was using proc_macro or proc_macro2 before.

@pietroalbini
Copy link
Member

FYI, seems like the meta crate on crates.io is squatted.

@Centril
Copy link
Contributor

Centril commented Sep 14, 2018

@pietroalbini yup, we took that as a sign from above that we should have the meta crate.

@sanmai-NL
Copy link

sanmai-NL commented Sep 21, 2018

@james-darkfox: that works for me too. See #54392.

@pnkfelix
Copy link
Member

visited for triage. @eddyb says "only the lint is missing now."

@pnkfelix
Copy link
Member

assigning to self to see about this lint.

@pnkfelix pnkfelix self-assigned this Sep 27, 2018
@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 27, 2018

Wait, could someone remind why adding std into extern prelude for no_std crates is a good idea at all?
In other words, why the last point in #54116 was implemented?

The original issue was about imports being able to access crates not from extern prelude.
That issue was fixed by implementing the first two points in #54116, but then reintroduced again by the third point.

What is the problem with the previous "whitelisting" scheme in which #[std] (I'll use to denote the opposite of #[no_std] in some way) introduces std into the extern prelude and #[core] introduces core into the extern prelude?
(It also could be implemented in the library unlike the current scheme that needs hardcoding in the compiler.)

cc @eddyb

@petrochenkov
Copy link
Contributor

#54116 refers to #53166 (comment), but it doesn't list motivation.

@aturon
Copy link
Member

aturon commented Sep 27, 2018

@petrochenkov I think part of the issue here is that some crates use #[no_std] but then go on to use std based on a feature flag they provide to their clients. I believe this happens today using an explicit extern crate std. But the remaining code wants to use paths like std::mem rather than crate::std::mem etc.

To support this existing pattern, we need some way to make std available as an external crate even in no_std mode. The proposal was then to just always have core and std available, and use a lint to detect problems in cases you're trying to actively avoid std.

An alternative might be to have another attribute that serves to "cancel out" #[no_std] (by bringing in std to the extern prelude).

@sanmai-NL
Copy link

@aturon: I question whether those maintainers are set on keeping that pattern or whether they would understand this needs to be fixed. I expect the latter. Again, some breaking change that is acceptable for the 2018 edition?

@petrochenkov
Copy link
Contributor

@aturon
#54647 is a very similar issue to what you are describing, but it cannot be solved by hardcoding in the compiler.
Perhaps we need a way to add custom names into the extern prelude from inside the crate itself, we can even reuse extern crate for that.

@petrochenkov
Copy link
Contributor

To support this existing pattern, we need some way to make std available as an external crate even in no_std mode.

#54658 implements this idea by reusing extern crate items.

@pnkfelix
Copy link
Member

pnkfelix commented Oct 4, 2018

(it looks like I should not assign high-priority to the proposed lint until after the lang team has come to a decision regarding PR #54658.)

@daboross
Copy link
Contributor

daboross commented Oct 11, 2018

std is currently usable in 2015 edition crates on nightly. Is this intentional, and if not, should I create another issue or is commenting here on this one OK?

I ran into this because I only test my crate's no-std functionality on rust nightly, and nightly happily accepts std:: paths in my rust 2015 edition crate.

Test case:

#![no_std]

#[derive(Debug)]
pub struct Xxx;

impl core::fmt::Display for Xxx {
    fn fmt(&self, f: &mut core::fmt::Formatter) -> core::fmt::Result {
        f.write_str("xxx")
    }
}

impl std::error::Error for Xxx {}

Playground: https://play.rust-lang.org/?gist=fc27ec823582d16044559c4010a75c7c&version=nightly&mode=debug&edition=2015


Regarding the issue itself, is there any reason we don't just get people to use #![cfg_attr(not(feature = "std"), no_std)]? Seems much more testable than silently reintroducing std if even one line accidentally depends on it. That is not very maintainable.

daboross added a commit to daboross/rust-throw that referenced this issue Oct 11, 2018
This should rightfully have been caught by the CI in the original PR,
but alas it was not. My bad for not seeing it manually.

See rust-lang/rust#53166 for issue on this
kind of mistake not causing an error like it used to.
@petrochenkov
Copy link
Contributor

This is going to be fixed once #54671 lands.

@pnkfelix
Copy link
Member

@daboross if I understand correctly, once #54671 lands, you should see the errors you expect from the code you posted

bors added a commit that referenced this issue Oct 17, 2018
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 added a commit that referenced this issue Oct 24, 2018
Add `extern crate` items to extern prelude

With this patch each `extern crate orig_name as name` item adds name `name` into the extern prelude, as if it was passed with `--extern`.

What changes this causes in practice?
Almost none! After all, `--extern` passed from Cargo was supposed to replace `extern crate` items in source, so if some code has `extern crate` item (or had it on 2015 edition), then it most likely uses `--extern` as well...

... with exception of a few important cases.

- Crates using `proc_macro`. `proc_macro` is not passed with `--extern` right now and is therefore not in extern prelude.
Together with 2018 edition import behavior this causes problems like #54418, e.g.
    ```rust
    extern crate proc_macro;
    use proc_macro::TokenStream;
    ```
    doesn't work.
It starts working after this patch.

- `#[no_std]` crates using `std` conditionally, like @aturon described in #53166 (comment), and still wanting to write `std` instead of `crate::std`. This PR covers that case as well.
This allows us to revert placing `std` into the extern prelude unconditionally, which was, I think, a [bad idea](#53166 (comment)).

- Later `extern crate` syntax can be extended to support adding an alias to some local path to extern prelude, as it may be required for resolving #54647.

Notes:
- Only `extern crate` items from the root module added to the prelude, mostly because this behavior for items from inner modules would look very strange, rather than for technical reasons.
This means you can opt out from the prelude additions with something like
    ```rust
    mod inner {
        pub(crate) extern crate foo;
    }
    use inner::foo;
    ```
- I haven't updated logic for 2018 import canaries to work fully correctly with this. The cases where it matters are pretty exotic (the `extern crate` item must be "sufficiently macro expanded") and I'd rather spend the time on eliminating the canaries entirely.
bors added a commit that referenced this issue Oct 24, 2018
Add `extern crate` items to extern prelude

With this patch each `extern crate orig_name as name` item adds name `name` into the extern prelude, as if it was passed with `--extern`.

What changes this causes in practice?
Almost none! After all, `--extern` passed from Cargo was supposed to replace `extern crate` items in source, so if some code has `extern crate` item (or had it on 2015 edition), then it most likely uses `--extern` as well...

... with exception of a few important cases.

- Crates using `proc_macro`. `proc_macro` is not passed with `--extern` right now and is therefore not in extern prelude.
Together with 2018 edition import behavior this causes problems like #54418, e.g.
    ```rust
    extern crate proc_macro;
    use proc_macro::TokenStream;
    ```
    doesn't work.
It starts working after this patch.

- `#[no_std]` crates using `std` conditionally, like @aturon described in #53166 (comment), and still wanting to write `std` instead of `crate::std`. This PR covers that case as well.
This allows us to revert placing `std` into the extern prelude unconditionally, which was, I think, a [bad idea](#53166 (comment)).

- Later `extern crate` syntax can be extended to support adding an alias to some local path to extern prelude, as it may be required for resolving #54647.

Notes:
- Only `extern crate` items from the root module added to the prelude, mostly because this behavior for items from inner modules would look very strange, rather than for technical reasons.
This means you can opt out from the prelude additions with something like
    ```rust
    mod inner {
        pub(crate) extern crate foo;
    }
    use inner::foo;
    ```
- I haven't updated logic for 2018 import canaries to work fully correctly with this. The cases where it matters are pretty exotic (the `extern crate` item must be "sufficiently macro expanded") and I'd rather spend the time on eliminating the canaries entirely.
bors added a commit that referenced this issue Oct 25, 2018
Add `extern crate` items to extern prelude

With this patch each `extern crate orig_name as name` item adds name `name` into the extern prelude, as if it was passed with `--extern`.

What changes this causes in practice?
Almost none! After all, `--extern` passed from Cargo was supposed to replace `extern crate` items in source, so if some code has `extern crate` item (or had it on 2015 edition), then it most likely uses `--extern` as well...

... with exception of a few important cases.

- Crates using `proc_macro`. `proc_macro` is not passed with `--extern` right now and is therefore not in extern prelude.
Together with 2018 edition import behavior this causes problems like #54418, e.g.
    ```rust
    extern crate proc_macro;
    use proc_macro::TokenStream;
    ```
    doesn't work.
It starts working after this patch.

- `#[no_std]` crates using `std` conditionally, like @aturon described in #53166 (comment), and still wanting to write `std` instead of `crate::std`. This PR covers that case as well.
This allows us to revert placing `std` into the extern prelude unconditionally, which was, I think, a [bad idea](#53166 (comment)).

- Later `extern crate` syntax can be extended to support adding an alias to some local path to extern prelude, as it may be required for resolving #54647.

Notes:
- Only `extern crate` items from the root module added to the prelude, mostly because this behavior for items from inner modules would look very strange, rather than for technical reasons.
This means you can opt out from the prelude additions with something like
    ```rust
    mod inner {
        pub(crate) extern crate foo;
    }
    use inner::foo;
    ```
- I haven't updated logic for 2018 import canaries to work fully correctly with this. The cases where it matters are pretty exotic (the `extern crate` item must be "sufficiently macro expanded") and I'd rather spend the time on eliminating the canaries entirely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests