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

pub(in) visibility doesn't allow `use`d paths, unless macro-expanded #60552

Open
qnighy opened this issue May 5, 2019 · 2 comments

Comments

Projects
None yet
3 participants
@qnighy
Copy link
Contributor

commented May 5, 2019

Paths usually respect use aliases of modules, but paths in pub(in) not.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a6c5906c564f63850530fcd28c64379c

mod m1 {
    pub(in crate::m2) fn f() {}
    //~^ERROR cannot find module `m2` in module `crate`
}
use m1 as m2;

This seems to be a matter of the order of evaluation, hence the following variant compiles:

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=a1b35f783b9c0aa35e58df3b7d4f41d5

mod m1 {
    crate::id! {
        pub(in crate::m2) fn f() {}
    }
}
use m1 as m2;

#[macro_export]
macro_rules! id {
    ($($x:tt)*) => { $($x)* };
}

I expect these behaviors to match.

@Centril

This comment has been minimized.

Copy link
Member

commented May 5, 2019

@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented May 5, 2019

It is assumed that PATH already exists when we are resolving pub(in PATH), otherwise it would hit the "visibility can only refer to parent modules" error.

Yes, this shortcut doesn't always work if you use aliases to parent modules, it also leads to suboptimal diagnostics in other cases (there are existing issues about that, but I can't find them right now).
However, resolving paths in pub(in PATH) properly by integrating them into iterative import-resolution/expansion algorithm is a large work for very little gain, so nobody got to fixing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.