Skip to content

WIP Improved support for visibility restrictions #88

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 13, 2017
Merged

WIP Improved support for visibility restrictions #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 15 additions & 43 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,24 +116,12 @@ pub use core::ops::Deref as __Deref;
#[cfg_attr(feature="nightly", allow_internal_unstable)]
#[doc(hidden)]
macro_rules! __lazy_static_internal {
($(#[$attr:meta])* static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@PRIV, $(#[$attr])* static ref $N : $T = $e; $($t)*);
};
($(#[$attr:meta])* pub(in $pub_in:path) static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@PUB_IN, $pub_in, $(#[$attr])* static ref $N : $T = $e; $($t)*);
};
($(#[$attr:meta])* pub static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@PUB, $(#[$attr])* static ref $N : $T = $e; $($t)*);
};
(@PUB_IN, $pub_in:path, $(#[$attr:meta])* static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@MAKE TY, PUB_IN, $pub_in, $(#[$attr])*, $N);
// optional visibility restrictions are wrapped in `()` to allow for
// explicitly passing otherwise implicit information about private items
($(#[$attr:meta])* ($($vis:tt)*) static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@MAKE TY, $(#[$attr])*, ($($vis)*), $N);
__lazy_static_internal!(@TAIL, $N : $T = $e);
__lazy_static_internal!($($t)*);
};
(@$VIS:ident, $(#[$attr:meta])* static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@MAKE TY, $VIS, $(#[$attr])*, $N);
__lazy_static_internal!(@TAIL, $N : $T = $e);
__lazy_static_internal!($($t)*);
lazy_static!($($t)*);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is correct, and why previously call to __lazy_static_internal! macro was put in here. This also somewhat duplicated the code. Can someone look at it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of a deeper reason for the duplication. As far as I can remember, when the internal macro split from the public one, this just happend with the idea that the publich one acts as the entry point for the internal one, which then just keeps to itself during expansion.

However, there is no reason for why it couldn't do the mutual recursion back into the public one instead, as done in this PR.

The only thing I can imagine is macro 1.0 weirdness in regard to visibility. As in, depending on how its imported, the internal macro might not see the public one during expansion. Not sure if this can happen here though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the information. I think that you should decide whether or not you like current implementation better than the previous one. If you do then you can merge the PR, otherwise let me know and I'll revert some changes I made and adapt it so that it plays well with all possible visibility restrictions.

};
(@TAIL, $N:ident : $T:ty = $e:expr) => {
impl $crate::__Deref for $N {
Expand All @@ -159,32 +147,15 @@ macro_rules! __lazy_static_internal {
}
}
};
(@MAKE TY, PUB, $(#[$attr:meta])*, $N:ident) => {
// `vis` is wrapped in `()` to prevent parsing ambiguity
(@MAKE TY, $(#[$attr:meta])*, ($($vis:tt)*), $N:ident) => {
#[allow(missing_copy_implementations)]
#[allow(non_camel_case_types)]
#[allow(dead_code)]
$(#[$attr])*
pub struct $N {__private_field: ()}
$($vis)* struct $N {__private_field: ()}
#[doc(hidden)]
pub static $N: $N = $N {__private_field: ()};
};
(@MAKE TY, PUB_IN, $pub_in:path, $(#[$attr:meta])*, $N:ident) => {
#[allow(missing_copy_implementations)]
#[allow(non_camel_case_types)]
#[allow(dead_code)]
$(#[$attr])*
pub(in $pub_in) struct $N {__private_field: ()}
#[doc(hidden)]
pub(in $pub_in) static $N: $N = $N {__private_field: ()};
};
(@MAKE TY, PRIV, $(#[$attr:meta])*, $N:ident) => {
#[allow(missing_copy_implementations)]
#[allow(non_camel_case_types)]
#[allow(dead_code)]
$(#[$attr])*
struct $N {__private_field: ()}
#[doc(hidden)]
static $N: $N = $N {__private_field: ()};
$($vis)* static $N: $N = $N {__private_field: ()};
};
() => ()
}
Expand All @@ -193,13 +164,14 @@ macro_rules! __lazy_static_internal {
#[cfg_attr(feature="nightly", allow_internal_unstable)]
macro_rules! lazy_static {
($(#[$attr:meta])* static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@PRIV, $(#[$attr])* static ref $N : $T = $e; $($t)*);
};
($(#[$attr:meta])* pub (in $pub_in:path) static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@PUB_IN, $pub_in, $(#[$attr])* static ref $N : $T = $e; $($t)*);
// use `()` to explicitly forward the information about private items
__lazy_static_internal!($(#[$attr])* () static ref $N : $T = $e; $($t)*);
};
($(#[$attr:meta])* pub static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!(@PUB, $(#[$attr])* static ref $N : $T = $e; $($t)*);
__lazy_static_internal!($(#[$attr])* (pub) static ref $N : $T = $e; $($t)*);
};
($(#[$attr:meta])* pub ($($vis:tt)+) static ref $N:ident : $T:ty = $e:expr; $($t:tt)*) => {
__lazy_static_internal!($(#[$attr])* (pub ($($vis)+)) static ref $N : $T = $e; $($t)*);
};
() => ()
}
Expand Down
10 changes: 10 additions & 0 deletions tests/compile-fail/incorrect_visibility_restriction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// incorrect visibility restriction
#[macro_use]
extern crate lazy_static;

lazy_static! {
pub(nonsense) static ref WRONG: () = ();
//~^ ERROR incorrect visibility restriction
}

fn main() { }
14 changes: 14 additions & 0 deletions tests/compile-fail/static_is_private.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#[macro_use]
extern crate lazy_static;

mod outer {
pub mod inner {
lazy_static! {
pub(in outer) static ref FOO: () = ();
}
}
}

fn main() {
assert_eq!(*outer::inner::FOO, ()); //~ ERROR static `FOO` is private
}
10 changes: 10 additions & 0 deletions tests/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,26 @@ mod visibility {
static ref BAR: Box<u32> = Box::new(98);
}

pub mod inner {
lazy_static! {
pub(in visibility) static ref BAZ: Box<u32> = Box::new(42);
pub(crate) static ref BAG: Box<u32> = Box::new(37);
}
}

#[test]
fn sub_test() {
assert_eq!(**FOO, 0);
assert_eq!(**BAR, 98);
assert_eq!(**inner::BAZ, 42);
assert_eq!(**inner::BAG, 37);
}
}

#[test]
fn test_visibility() {
assert_eq!(*visibility::FOO, Box::new(0));
assert_eq!(*visibility::inner::BAG, Box::new(37));
}

// This should not cause a warning about a missing Copy implementation
Expand Down