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

Allow pub use re-imports/exports of already privately imported symbols #1285

Closed
felixc opened this issue Sep 18, 2015 · 11 comments
Closed

Allow pub use re-imports/exports of already privately imported symbols #1285

felixc opened this issue Sep 18, 2015 · 11 comments
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.

Comments

@felixc
Copy link

felixc commented Sep 18, 2015

Currently, if all the symbols in a crate have already been privately imported via use foo::*, it is not possible to publicly re-export any of them via pub use foo::Bar.

The use case where this comes up is in writing a wrapper library. All of the code will be dealing with types and functions from one crate, so it is desirable to wildcard-import everything. However, it may also be necessary to re-export some of those types, which is currently not possible.

In code, it would be nice if this worked:

use foo::*;
pub use foo::Bar;

Whereas currently it fails with error: a type named 'Bar' has already been imported in this module [E0252].

@blaenk
Copy link
Contributor

blaenk commented Sep 18, 2015

Yeah this seems like it would complement glob imports nicely. It's a shame that it's not possible to use glob imports when you want to selectively re-export some of the items.

@felixc
Copy link
Author

felixc commented Sep 18, 2015

It was pointed out on IRC that it is possible the import resolver is being naïve here: it is checking for multiple imports of the same name to prevent one shadowing the other, but in this case, both imports actually point to the same type. That does not seem like the error that the check is intended to catch.

@codyps
Copy link

codyps commented Sep 18, 2015

Would be similarly useful to allow publicly exposing items generated by a macro (where that macro may not necessarily support appending pub). And would let macro writers avoid needing to handle pub.

@felixc
Copy link
Author

felixc commented Sep 18, 2015

@jmesmon Thanks for the pointer to that use case; I wasn't aware of it. Do you happen to know offhand of any specific examples where either the macro makes it impossible to publicly expose something, or where the macro author has to go to extra effort to make it work? If this ends up turning into an RFC it would be nice to be able to point to real-world examples of pain, but as I said, I'm not familiar with that use case.

@codyps
Copy link

codyps commented Sep 18, 2015

@felixc nix-rust/nix#184 was the case I ran into recently.

@felixc
Copy link
Author

felixc commented Sep 18, 2015

@jmesmon Thank you!

At this point, unless someone in the know steps in and confirms that this is a bug in the import resolver and it should not error on duplicate names that correspond to the same item (and thus this change can happen without much process/formality), I'll take the time to write up a draft RFC over the next few days.

@Kimundi
Copy link
Member

Kimundi commented Sep 21, 2015

The whole interaction of glob imports with regular imports is basically a bug, with a plan by @nikomatsakis to fix their semantic by allowing glob imports to be shadowable by non-glob imports. (However, no RFC has yet been submitted about that)

Under that semantic, the pub use foo::Name; would likely override the use foo::*; import for the Name item, allowing this usecase.

@felixc
Copy link
Author

felixc commented Sep 24, 2015

@Kimundi Thanks for the pointer! I'm glad to hear it's already under consideration.

Should this bug be closed in favour of the larger meta-bug? Or is there something one could do here to encourage/accelerate that related work? Would an RFC for this sub-feature help get this small piece of the problem solved sooner?

@Kimundi
Copy link
Member

Kimundi commented Sep 25, 2015

I think this issue should remain open for now, but I'm not sure a RFC would help apart from drawing more attention.

The issue is that this would be more or less an ad-hoc addition to the name resolution system, rather than something more general that integrates well into all aspects of it.

In terms of giving support to make this happen sooner, I'm not sure. That metabug is, for now, just my personal opinion on how to fix those things, I guess you could comment on that if you agree with it.

Apart from that, well there's only waiting until the matter got enough attention from core devs to actually start talking about wether to implement those changes or not, at which point there would be a more detailed RFC to comment under. :)

@felixc
Copy link
Author

felixc commented Sep 26, 2015

Sounds good — I'll leave this open for now as a record of the points discussed here, as well as to attest to the support for the feature from multiple people. Thanks everyone!

@nrc nrc added the T-lang Relevant to the language team, which will review and decide on the RFC. label Aug 22, 2016
@nrc
Copy link
Member

nrc commented Aug 22, 2016

Fixed by #1560.

@nrc nrc closed this as completed Aug 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

5 participants