-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Tracking issue for macro_reexport
feature
#29638
Comments
Any opinions on whether this should be stabilized? I would love to see it stabilized. Currently, a crate A which exports a macro whose expansion references a macro from crate B cannot express that dependency: it is pushed onto the user of crate A, even though otherwise they wouldn't have to know about the implementation detail. If crate A could reexport the macro from crate B, it would be a better situation. |
cc @rust-lang/lang, particularly @nrc. In the long run this will be able to go through the normal Nominating for discussion in next lang meeting. |
Hear ye, hear ye. This issue is hereby promoted to final comment period. The discussion is whether to stabilize this feature beginning in the next release cycle. |
I think we should stabilise. As @aturon says, eventually it should be replaced, but having it in the short/medium term is a big win. |
Some serious bugs and downsides of
I personally see this as a half-baked feature that was just nice to reduce duplication in std/core, it doesn't seem like it fits the normal high standard we might have for language features. There's certainly an argument that I'd be fine deprecating and removing this while just copying the macros between core/std to have the same source. |
The first bug seems possible to fix -- if the mangled source of a macro is stored in crate metadata, the unmangled source can be too. I agree the second one is a downside, but it can be mitigated by |
We should fix both the bugs that @alexcrichton points out before stabilising (I was not aware of them). Are there issues filed for those? |
Unfortunately not that I know of :( Sorry I meant to write these thoughts down when the initial tracking issue was made, but I guess I forgot... |
The mangled-source issue is #20923. |
Making Also, if |
After discussion in the @rust-lang/lang meeting, we decided not to stabilize this feature just yet (and hopefully not ever). The key points of discussion already appear in this thread:
|
I agree that it's only practical for a facade pattern, but sometimes that's I'm confused by your last bullet. Can you elaborate on how Cargo features
|
On Sat, Apr 23, 2016 at 08:00:26AM -0700, Alex Burka wrote:
I meant that cargo features can be used in preference to the facade pattern. |
Sorry for being thick... I just don't see what the relation is between On Wed, Apr 27, 2016 at 1:55 PM, Niko Matsakis notifications@github.com
|
It can also be useful to re-export things from crates maintained by other people / in other repositories. |
Since 1.15.0, extern crate somelib;
pub use somelib::*; As far as I’m concerned this removes the need for this feature. Test case: set -e
cargo new aa
cargo new bb
cargo new cc
echo 'bb = {path = "../bb"}' >> aa/Cargo.toml
echo 'cc = {path = "../cc"}' >> bb/Cargo.toml
echo '#[macro_use] extern crate bb;' > aa/src/lib.rs
echo '#[test] fn a() { assert_eq!(c!(), 42) }' >> aa/src/lib.rs
echo '#[macro_use] extern crate cc;' > bb/src/lib.rs
echo 'pub use cc::*;' >> bb/src/lib.rs
echo '#[macro_export] macro_rules! c { () => { 42 } }' > cc/src/lib.rs
(cd aa && cargo +1.15.0 test)
rm -r aa bb cc |
I'm not sure whether that is a full replacement of the feature, as you can refine by doing |
Isn’t that a bug to fix? |
lol apparently the reverse is true, the glob use feature is a bug, and should be feature gated. Also cc #35896 |
Maybe this should have been feature-gated but it hasn’t been for three release cycles. |
Is this the tracking issue for use_extern_macros or is there a separate tracking issue for that? How are these two features different / related? |
@withoutboats the two features are different. I'd say they both implement the same thing but in different ways. macro_reexport: #![crate_type = "dylib"]
#![feature(macro_reexport)]
#[macro_reexport(reexported)]
#[macro_use] #[no_link]
extern crate macro_reexport_1; use_extern_macros: #![feature(use_extern_macros)]
extern crate two_macros;
::two_macros::macro_one!();
two_macros::macro_one!();
mod foo { pub use two_macros::macro_one as bar; } The tracking issue for I would guess that the intention is that |
@jseyfried From the discussion here and in #40768 (comment) it sounds like you think the feature is ready and are in favor of stabilizing. Is that correct? |
|
@cramertj What I am in favor of stabilizing is extern crate macros;
pub use macros::{foo, bar};
|
@jseyfried As far as I can tell, there is not a separate tracking issue for |
This is a bit hax: it uses the macros directly from the upstream crate rather than a more elegant way of re-exporting them. Doing it better seems blocked on this issue, which isn't likely to happen: rust-lang/rust#29638
- The `macro_reexport` feature was removed and superseded by `use_extern_macros`. <rust-lang/rust#29638> - Switch to the version of `stdsimd` provided directly by the toolchain rather than one distributed via crates.io or github.io - `never_type` is unstable again <rust-lang/rust#35121> - `cfg_target_feature` is stable since 1.27.0 - `target_feature` is stable since 1.27.0
- The `macro_reexport` feature was removed and superseded by `use_extern_macros`. <rust-lang/rust#29638> - Switch to the version of `stdsimd` provided directly by the toolchain rather than one distributed via crates.io or github.io - `never_type` is unstable again <rust-lang/rust#35121> - `cfg_target_feature` is stable since 1.27.0 - `target_feature` is stable since 1.27.0
This is a bit hax: it uses the macros directly from the upstream crate rather than a more elegant way of re-exporting them. Doing it better seems blocked on this issue, which isn't likely to happen: rust-lang/rust#29638
Tracking for stabilization of
#[macro_reexport]
.The text was updated successfully, but these errors were encountered: