resolve: Privacy rules for re-exports can be too restrictive

[jseyfried]

mod foo {
    mod bar { pub fn f() {} }
    pub fn bar() {}
}

// This should re-export bar in the value namespace; now, it complains that the module `bar` is private.
pub use foo::bar; 

// This re-exports bar in the value namespace as expected.
pub use foo::*;

fn main() { bar::f() }

[petrochenkov]

Well, use foo::bar unlike use foo::* does try to access a private name.
Personally, I wouldn't bother to change this (unless it would follow logically from some grander scheme), the practical impact is negligible.

I have more interesting examples though, in which use statement simultaneously accesses two things with different properties. I'll post them when I get home.

[jseyfried]

I agree that the practical impact is very small, but I do think it is inconsistent to be able to re-export a pub item with a glob but not by name.

I think of * as a pattern that matches any name in any namespace and bar as a pattern that matches the name bar in any namespace. When we pub use a glob pattern, we import everything pub that matches the pattern, so I would expect a pub use of a name to import everything pub with that name.

I'm definitely interested in the examples -- not sure what properties means in this context.

[petrochenkov]

I think of * as a pattern that matches any name in any namespace and bar as a pattern that matches the name bar in any namespace.

In this case use foo::bar would be a noop and not an error when both bars are private or don't exist. That would be pretty strange.

When we pub use a glob pattern, we import everything pub that matches the pattern

I think this is going to change when proper glob shadowing is implemented to support cases like

struct S;
mod m {
    use super::*;

    let s = S; // OK
}

but there are some details, I'll elaborate later.

[jseyfried]

In this case use foo::bar would be a noop and not an error when both bars are private

Good point. To be fair though, it would be an unused import warning if a glob pattern didn't match anything. It actually might be a good idea to not allow an empty glob import -- I don't think anyone would do that intentionally.

I think this is going to change when proper glob shadowing is implemented

mod foo {
    pub fn f() {}
    fn g() {}
}

Right now pub use foo::* is equivalent to pub use foo::f. Are you saying that pub use foo::* would change to be equivalent to pub use foo::f; use foo::g?
If so, I would expect pub use foo::bar from my original example to re-export bar in the value namespace and privately use it in the type namespace.

[jseyfried]

There's also precedent here -- this compiles:

mod foo {
    mod bar { pub fn f() {} }
    pub fn bar() {}
}

// This effectively imports `foo` in the value namespace. I say "effectively" because
// trying to use the imported name `foo` in the type namespace is a privacy error,
// not an unresolved name error.
use foo::bar;

fn main() {
    bar();
    // bar::f(); // If this line were uncommented, the import of `foo` would be a privacy error.
}

[petrochenkov]

not sure what properties means in this context.

DefModifiers mostly, you take two entities with different modifiers and try to use it with use foo::type_value, pub use foo::type_value, use foo::* and pub use foo::*. If one of the modifiers lead to an error when used in such way and other doesn't, then things become interesting, and in different ways (you have already write out several interesting cases above). I'll try to write out all these cases methodically and suggest my preferred compiler behavior in each case.

[petrochenkov]

Theoretically, an import with n segments can resolve to not even 2, but 2ⁿ entities, because each segment can bifurcate into two namespaces.
Thankfully, items in value namespace can't have children currently.

Note: this currently compiles, but I'm not sure it should

mod m {
    pub fn f() {}
}

use m::f::{self};

it allows to observe one initial step of the bifurcation mentioned above

mod m {
    pub fn f() {}
    pub mod n {}
}

use m::f::{self};

[jseyfried]

Interesting. I think that example should definitely not compile.
To avoid the bifurcation, I think it would be a good idea consider of all segments but the last in a path to match only the type namespace, so the f in m::f::{self} should not be able to resolve to a value.

[petrochenkov]

consider of all segments but the last in a path to match only the type namespace

This seems to be already done in globs - use m::f::*; fails with an error "Could not find f in m"

[petrochenkov]

I'll try to write out all these cases methodically and suggest my preferred compiler behavior in each case.

After some experimenting, I think there are only two basic scenarios, and there's a simple scheme covering both concrete imports foo::bar, glob imports foo::* and even something highly hypothetical like foo::[Rr]ege[x]+ (I assume a future-compatible setup in which private imports are proper items, globs can import private items etc.)

1. use

Isolated use foo::bar or foo::* is "desugared" into a candidate list

use foo::bar_1;
use foo::bar_2;
...
use foo::bar_n;

If use foo::bar_i; is ill-formed for some reason it's removed from the list. If the list is empty after the removal of all ill-formed use foo::bar_i;, then the original import is ill-formed and an error is reported.

2. pub use

Isolated pub use foo::bar or pub foo::* is "desugared" into a candidate list

pub use foo::bar_1;
pub use foo::bar_2;
...
pub use foo::bar_n;

If pub use foo::bar_i; is ill-formed, then it's lowered to use foo::bar_i;. If use foo::bar_i; is ill-formed too, then it's removed from the list. If the list contains 0 pub candidates after all the lowerings and removals, then the original import is ill-formed and an error is reported.

---

This scheme can be straightforwardly extended to support pub(path) with both use and pub use cases unified:

Isolated pub(path) use foo::bar or pub(path) foo::* is "desugared" into a candidate list

pub(path) use foo::bar_1;
pub(path) use foo::bar_2;
...
pub(path) use foo::bar_n;

The visibilities pub(path) are "decremented" until they are compatible with the imported item's visibilities pub(path_i). Candidates with visibilities not compatible with the current module are removed from the list. If the list contains 0 pub(path) candidates after all the decrements and removals, then the original import is ill-formed and an error is reported.

[jseyfried]

Interesting, I like these semantics.

I was envisioning something similar except without "decrementing" the visibilities. pub use foo::[pattern]; under your semantics would be equivalent to pub use foo::[pattern]; use foo::[pattern]; under my semantics since if a name is imported twice and both imports refer to the same item, the more visible import would win. (EDIT: I think this is a bad idea, especially after RFC 1422)

[jseyfried]

After #31726, it will be straightforward to implement the setup you describe (i.e. private imports are proper items, globs can import private items, unused ambiguous glob-imported names are OK, multiple imports of the same item are OK, items and named imports can shadow globs).

The current plan seems to be for the RFC proposing these changes to also propose a macros 2.0 system with new hygiene rules and the ability to import and use macros like any other item (changes that I support).

However, I think the new import semantics should get their own RFC before (or beside) macros 2.0 since the new import semantics are much easier to implement, backwards incompatible (theoretically -- hopefully not in practice), more immediately useful (imo), and already are partially implemented by accident. What do you think?

[petrochenkov]

However, I think the new import semantics should get their own RFC before (or beside) macros 2.0 since the new import semantics are much easier to implement, backwards incompatible (theoretically -- hopefully not in practice), more immediately useful (imo), and already are partially implemented by accident. What do you think?

Sure, if you are already working on it, then why wait for macros 2.0, @nrc can review the RFC and ensure it's compatible with the new macro system.

[petrochenkov]

What I don't like in this #31783 (comment) scheme (and in the current scheme as well) is that resolution depends on privacy. I.e. to determine the import set and well-formedness for (1)

pub use(path1) foo::*; // (1)

mod foo {
    pub(path2) use bar::*;
    pub(path3) struct Baz;
}

we need to resolve paths path1, path2, path3 first, which in their turn can depend on the resolution of (1).

Can we come up with a scheme not using privacy information in any way in build_reduced_graph/resolve_imports stages? Could these two stages see the crate as completely public with all the import resolution sets and "conflicts" preserved and resolved later during name lookup (or/and in a separate pass) through shadowing/prioritization and filtering out privacy-violating resolutions?

[jseyfried]

// If the type `root::bar` is `root::foo::bar -> root`, then `pub(root::bar)` is public and the glob is shadowed.
// If the type `root::bar` is `root::foo`, then `pub(root::bar)` is private to `foo` and the glob is not shadowed.
mod root {
    mod glob_imported {
        pub use root::foo as bar;
    }
    mod foo {
        pub fn bar() {}
        pub(root::bar) use root as bar;
    }
    use root::glob_imported::*;
    use root::foo::bar;
}

// The same thing happens here, except that the bindings are not stable in either
// configuration instead of being stable in both configurations.
mod root {
    mod glob_imported {
        pub use root as bar;
    }
    mod foo {
        pub fn bar() {}
        pub(root::bar) use root::foo as bar;
    }
    use root::glob_imported::*;
    use root::foo::bar;
}

[jseyfried]

It would be doable to implement such a scheme, but it would add quite a bit of complexity.
I think that having multiple consistent/stable ways to resolve a crate is bad idea, even if the compiler can select a canonical one.

[jseyfried]

Maybe we could only allow pub to be annotated with a path for which no segment resolves to an import (i.e. the "canonical paths", plus possibly a leading self or supers).

It seems excessive to allow all paths since all we need to express is a De Bruijn index -- the paths self, super, super::super, ... alone would be sufficient.

[petrochenkov]

I expected shadowing to precede privacy checks.
I.e. a crate is resolved as if it were completely public and all the conflicts are resolved based on shadowing. Privacy violations are checked after that.

But it still leaves "glob vs glob" conflicts. And they need to be resolved based on privacy, otherwise using two globs in one module could lead to unpredictable conflicts due to implementation details.
And now suppose we have pub(path) use something where path represents a "glob vs glob" conflict...

Yeah, it seems like restricting paths in pub(path) to be "canonical paths" (or at least not "glob vs glob" conflicts) would be the best solution.

[jseyfried]

@petrochenkov Thinking about this some more, I strongly prefer your semantics over my alternative.

Under your semantics, the following would compile today, right?

pub fn foo() {}
mod foo { pub fn f() {} }

mod bar {
    pub use foo; // This publicly imports `foo` in the value namespace and privately imports `foo` in the type namespace.
    fn g() { foo::f() }
}

fn main() {
    bar::foo();
}

[petrochenkov]

the following would compile today, right?

Yep.

Edit: And with pub(restricted) private becomes pub(self), so mod foo becomes pub(crate) and bar::foo in the type namespace becomes pub(crate) as well.

[petrochenkov]

This was fixed by stabilizing RFC 1560.
@jseyfried please reopen if something is missing.