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

extern crate is always treated as pub within the crate, but is not visible outside it; pub extern crate makes no difference #26775

Closed
brson opened this issue Jul 4, 2015 · 17 comments · Fixed by #31362
Assignees
Labels
A-resolve Area: Name resolution A-visibility Area: Visibility / privacy P-high High priority T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Jul 4, 2015

Two examples:

#![feature(core)]
#![crate_type = "lib"]
pub extern crate core;

That is accepted but core cannot be accessed from outside the crate.

#![feature(core)]
#![crate_type = "lib"]

mod foo {
    extern crate core;
    // behaves identically
    // pub extern crate core;
}

pub use foo::core;

That is accepted and reexports core.

In other words, extern crate is always public to other modules, but can never be made public to other crates.

There are no tests for pub extern crate so I suspect this is not intentional.

@brson brson added I-wrong A-parser Area: The parsing of Rust source code to an AST T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 4, 2015
@eddyb
Copy link
Member

eddyb commented Jul 5, 2015

Also see #21757.

@nikomatsakis
Copy link
Contributor

triage: P-high

We should nail this down. I'm assigning this to @nrc since iirc he last touched the privacy code. ;)

@rust-highfive rust-highfive added P-high High priority and removed I-nominated labels Jul 9, 2015
@pnkfelix
Copy link
Member

pnkfelix commented Jul 9, 2015

(any reason we can't just ... make pub extern crate a parse error ... ?)

Update: (with a warning cycle in the release series, of course)

Update 2: Hmm, I guess I missed the point that it can also be reexported, so making pub extern crate illegal is probably not the right call. (that plus RFC 385 explicitly said it was making pub extern crate legal.)

@brson
Copy link
Contributor Author

brson commented Aug 4, 2015

@pnkfelix Making it illegal might still be a reasonable stop-gap if it looks like we won't fix the problem for a while.

nrc added a commit to nrc/rust that referenced this issue Sep 18, 2015
bors added a commit that referenced this issue Sep 19, 2015
brson pushed a commit to brson/rust that referenced this issue Oct 16, 2015
@nikomatsakis
Copy link
Contributor

Can this be made into a hard error on nightly now?

@nikomatsakis
Copy link
Contributor

Should we do a crater run? (with this marked as error)

Advice: if we do, please only make it an error if cap-lints is not provided.

@alexcrichton
Copy link
Member

This has reached stable now, so I'd at least be fine going to a hard error! (pending crater)

@eddyb
Copy link
Member

eddyb commented Nov 6, 2015

See #29654 for a more breakage-prone issue with extern crate privacy (more so than pub extern crate, which is pretty harmless atm).

@nrc nrc changed the title 'pub extern crate' is accepted and behaves badly extern crate is always treated as pub within the crate, but is not visible outside it; pub extern crate makes no difference Nov 16, 2015
@nrc nrc added A-resolve Area: Name resolution A-visibility Area: Visibility / privacy and removed A-parser Area: The parsing of Rust source code to an AST labels Nov 16, 2015
@nrc
Copy link
Member

nrc commented Nov 16, 2015

I was looking at this today. Let me get specific with the issues:

  1. extern crate is always treated as public within a crate
  2. pub extern crate was no more public, currently it warns on use, the plan was to make it an error and fix later.
  3. pub extern crate does not export outside the crate.

The RFC is a little vague - it states that extern crate should work in sub-modules as well as the crate root and that pub extern crate should make it public, which includes re-exporting it to other crates.

So, I thought, lets make extern crate just like use with respect to privacy. This does not work. Let me summarise how use/pub use work.

  1. use imports a name into a module
  2. pub use imports a name into a module and re-exports it as a name like items declared in the module. Note that in step one we don't make the imported name available as a private name, it is not re-exported at all.
  3. I.e., privacy checking for imports is entirely a function of name resolution, not the privacy checking phase. So when we check that a path is accessible in privacy checking, we check only that the target is accessible, not that each segment is also accessible (because if any segment were not accessible, we could not name it). (Note that privacy of module 'literal's is checked slightly orthogonally to this).

If extern crate were to work like use, then a crate imported at the crate root would not be visible in child modules - clearly this breaks the world.

It seems the best solution is to treat extern crate more like inlining a module than importing a name. In this case, I think we get the expected semantics for pub and non-pub extern crate and the only back-compat hazard is where users have extern crate in a sub-module but refer to it from outside. However, this is not trivial because it doesn't fit with how we check privacy, iirc, we check from the target of a path until we either get to the 'caller' or to the global namespace, and error-ing out if we find a private module. With the extern crate change we have to check how the item was imported too. I think this is doable, but it's not as easy as I was expecting (also I don't understand the privacy code as well as name resolution).

Alternatively, we could probably leave things as is for now, perhaps carry on with the plan to make pub extern crate a hard error and fix this for 2.0 (since we want to re-architect name resolution any way, it might be easier to do later).

cc @petrochenkov since he has been looking at privacy.

@nrc
Copy link
Member

nrc commented Nov 16, 2015

and cc @rust-lang/lang

@Kimundi
Copy link
Member

Kimundi commented Nov 16, 2015

@nrc for what it's worth, the semantic I intended in the RFC was formulated under the wrong assumption that private use items exported a name from the module like other private items.

Since I see that aspect of use as another remaining inconsistency in the module system, and since it might change as part of a backwards compatible name resolution reachitecture, I'd be in favour of punting on resolving this for now until it is clear wether that change will happen or not. 😄


Some background:
The general vision I have for the module system is for all items (including use and extern crate) in a crate to behave identically in their interaction with it. That is to say, they all should be markable as public or private, and they all should follow the same privacy and reachability rules when referring to each other. If that vision is not achievable, I'd rather find a new consistent view than have stuff be partially implemented.

@nikomatsakis
Copy link
Contributor

@nrc and I discussed a bit today. My feeling is that:

  1. Like @Kimundi, I too want to make the semantics of use like all other items. But that seems like a bit of a distraction here. Whatever you think of use, you would probably agree that extern crate should act like other items in terms of being importable etc.
  2. It seems like the move to warn on pub extern crate may not have been correct, or was at least too broad:
    • we probably want to warn if you do pub extern crate at the root level
    • but probably not at other levels, even though the behavior at non-root levels IS what you'd expect from pub extern crate
    • we probably don't want to warn at non-root levels because we don't want people to remove the pub if they expect access from outside the current module (which I would think is unusual, though it may occur)
  3. I imagine that most people who put extern crate at any level other than the root intend for it to be privately used as an implementation detail. Anyway, it seems like we should fix this, but it also seems like a clear bug fix and hopefully one that won't lead to too much fallout.
  4. Maybe the new privacy pass will make this easier to fix? Unclear.

@SimonSapin
Copy link
Contributor

I’ve been using this pattern to simulate pub extern crate:

pub mod tendril {
    extern crate tendril;
    pub use tendril::*;
}

Manishearth added a commit to Manishearth/rust that referenced this issue Feb 25, 2016
…ty, r=nikomatsakis

This PR changes the visibility of extern crate declarations to match that of items (fixes rust-lang#26775).
To avoid breakage, the PR makes it a `public_in_private` lint to reexport a private extern crate, and it adds the lint `inaccessible_extern_crate` for uses of an inaccessible extern crate.

The lints can be avoided by making the appropriate `extern crate` declaration public.
posborne added a commit to posborne/nix-rust that referenced this issue Mar 4, 2016
With rust 1.7, the following warning was being emitted by the compiler:

    warning: `pub extern crate` does not work as expected and should
    not be used. Likely to become an error. Prefer `extern crate` and
    `pub use`.

Based on rust-lang/rust#26775 it appears that
this change is the current best approach for properly exporting the
libc dependency so it may be used outside of nix.

Signed-off-by: Paul Osborne <osbpau@gmail.com>
posborne added a commit to posborne/nix-rust that referenced this issue Mar 5, 2016
With rust 1.7, the following warning was being emitted by the compiler:

    warning: `pub extern crate` does not work as expected and should
    not be used. Likely to become an error. Prefer `extern crate` and
    `pub use`.

Based on rust-lang/rust#26775 it appears that
the warning in 1.7 which was to be escalated to an error is going away
but in older versions of rust it is still the case that `pub extern crate`
will not work as expected.  Instead, we use a somewhat creative hack to
export the libc root as a module in nix.  Down the line, it may make
sense to either eliminate the need to export libc (by chaning the ioctl
macros) or to move toward deprecated older versions of rustc.

Signed-off-by: Paul Osborne <osbpau@gmail.com>
posborne added a commit to posborne/nix-rust that referenced this issue Mar 11, 2016
With rust 1.7, the following warning was being emitted by the compiler:

    warning: `pub extern crate` does not work as expected and should
    not be used. Likely to become an error. Prefer `extern crate` and
    `pub use`.

Based on rust-lang/rust#26775 it appears that
the warning in 1.7 which was to be escalated to an error is going away
but in older versions of rust it is still the case that `pub extern crate`
will not work as expected.  Instead, we use a somewhat creative hack to
export the libc root as a module in nix.  Down the line, it may make
sense to either eliminate the need to export libc (by chaning the ioctl
macros) or to move toward deprecated older versions of rustc.

Signed-off-by: Paul Osborne <osbpau@gmail.com>
homu added a commit to nix-rust/nix that referenced this issue Mar 11, 2016
libc: re-export libc properly

With rust 1.7, the following warning was being emitted by the compiler:

    warning: `pub extern crate` does not work as expected and should
    not be used. Likely to become an error. Prefer `extern crate` and
    `pub use`.

Based on rust-lang/rust#26775 it appears that
this change is the current best approach for properly exporting the
libc dependency so it may be used outside of nix.

Signed-off-by: Paul Osborne <osbpau@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name resolution A-visibility Area: Visibility / privacy P-high High priority T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants