Support for additional access specifiers

[GabrielMajeri]

This pull request adds support for new access specifiers like pub (crate) or pub (in some_module).

This is currently a nightly only feature, so this pull request should not currently be merged.
It breaks all doctests and integration tests, since they'd need to manually enable #![feature(macro_vis_matcher)].

Fixes #72 (once merged).

[KodrAus]

Nice work! Thanks @GabrielMajeri. I'm not sure if we've got an idea of when macro_vis_matcher should stabilise yet, but hopefully there shouldn't be much churn here so I think it's safe to keep this around for a bit.

Also worth noting that once it does stabilise and we do merge then the minimum rustc version will be bumped.

[tamird]

This can go in behind a cargo feature, right?

[GabrielMajeri]

@tamird yes it can, I've now placed it behind the visibility Cargo feature.

[KodrAus]

Hmm, I'm not personally sold on the need for a cargo feature, at least not long-term. I get that bumping the minimum rustc version is potentially breaking, especially if we bump it to the latest stable right away. But since pub(restricted) is a fundamental language feature it seems weird to need a cargo feature to support a language feature in another language feature. So if we did want one now to minimise breakage then I'd be happy if we had a plan to make it the status-quo down the track.

Do you have any thoughts @alexcrichton?

[tamird]

I don't follow, @KodrAus. Using a cargo feature allows this to merge before the necessary bits are stabilized upstream. Bumping the minimum rust version is a separate discussion - there's no reason to hold this up on that decision.

[KodrAus]

Yep, sorry @tamird I was getting ahead of myself 😅 Feel free to ignore all of that until vis stabilises.

[alexcrichton]

Oh I figured we'd just add support when this is in stable rust, but I don't think it is?

[KodrAus]

Yeh it looks like it's still unstable.

I was originally thinking we shouldn't release with a cargo feature because then we'd have to maintain a fork of the macro, and people shouldn't need to use a feature to get pub(restricted) once it's stable.

But I think we could use a feature, and then when it does stabilise we could just make that feature a no-op, since the :vis macro version is essentially more general.

I guess the alternative is not use a feature and let this PR sit here waiting until it can work on stable, but I don't know when that would be.

[alexcrichton]

That's true yeah, I'd also be ok landing it behind an off-by-default feature.

[dtolnay]

I would prefer to do this in a way that immediately works on stable. Would something like this be possible?

/*
#![feature(macro_vis_matcher)]

macro_rules! m {
    ($vis:vis const $n:ident) => {
        $vis const $n: i32 = 0;
    };
}
*/

macro_rules! m {
    (const $n:ident) => {
        __impl_m!(() const $n);
    };
    (pub const $n:ident) => {
        __impl_m!((pub) const $n);
    };
    (pub ($($vis:tt)+) const $n:ident) => {
        __impl_m!((pub ($($vis)+)) const $n);
    };
}

macro_rules! __impl_m {
    (($($vis:tt)*) const $n:ident) => {
        $($vis)* const $n: i32 = 0;
    };
}

m!(const PRIV);
m!(pub(crate) const CRATE);
m!(pub const PUB);

fn main() {
    println!("{} {} {}", PRIV, CRATE, PUB);
}

[KodrAus]

Neat! What do you think @GabrielMajeri?

[GabrielMajeri]

@KodrAus I have created a new pull request for that (#135), and will keep this updated for when vis stabilizes.

[dtolnay]

Superseded by #135.