Amend RFC 48

[nikomatsakis]

The original definition of public items was based on reachability rather than simply checking whether the item is declared pub or not. The RFC also did not describe how privacy and impl definitions are related.

[Gankro]

Should this be "are not public"?

[SiegeLord]

The pub use section doesn't talk about glob re-exports. I assume they will continue to work, but only re-export the pub types? I.e. this won't be an error:

pub use barrier::*;

mod barrier
{
    pub struct A;
    struct B;
}

[glaebhoerl]

You mean "have to be public"?

[glaebhoerl]

My thinking was that trait impls do not create items which can be referenced in APIs. You refer to the trait and its methods; these are then resolved during typechecking (not name resolution!) to a particular impl (if possible), but this happens behind the scenes and is opaque to the code doing the referencing. Is this point of view Haskell-centric and not true in Rust?

[glaebhoerl]

This is... interesting. At first I was going to write:

For the same reason, it's not clear to me that this should be forbidden. Client code sees traits and their associated items; the typechecker sees the impls. Going with associated types syntax, client code can refer to the given type as fn some_func<T: PubTrait>(arg: T::AssocType); the fact that this may resolve to a private type doesn't seem significant, because it's abstract to some_func.

But, if you write fn other_func(arg: PubStruct::AssocType), that's no longer the case (this is very similar to pub type NotAssocType = PrivStruct). So I think you're in fact correct that this should be forbidden.

[glaebhoerl]

The reason I didn't (and still don't) think "is the item declared pub" is sufficient is that the logic of Rust's privacy rules is that you can see everything looking "outwards", but you can only see (through) pub things looking "inwards". Private modules, just like other private items, are an implementation detail of the containing module, and a public item in a private submodule is only public to the parent module, not to anyone else. (A private item in a private submodule is not even public to the parent module.) A public item in a private submodule is not materially different from a private item in the parent module as far as client code is concerned.

mod impl_ {
    pub struct Foo { ... }
}

pub type Bar = self::impl_::Foo;

If you can write this, then what have we actually gained? I suspect you could re-encode every otherwise forbidden example as public-thing-in-private-module this way.

[nikomatsakis]

On Thu, Aug 14, 2014 at 06:51:37PM -0700, Gábor Lehel wrote:

If you can write this, then what have we actually gained? I suspect
you could re-encode every otherwise forbidden example as
public-thing-in-private-module this way.

I realize now that the threat model you are concerned about is
different from what I am concerned about. I am concerned specifically
with people who are writing unsafe code or other code where knowing
what might be manipulated from outside the package is very
important.

I would like a guarantee that items declared as private are truly
private, meaning that if I search through that module (and
submodules), I can find every piece of code that directly manipulates
those items (i.e., calls a fn, or has access to an instance of a type,
etc).

In contrast, I am not actually concerned about people inconsistently
exposing public things, even though this can result in variables whose
types can only be inferred, not written. I agree that is a poor
interface design but it's not dangerous. (I should amend the
motivation in the pull request to reflect my view.)

Basically the guarantee I want to achieve is:

• If an item is not declared pub, the item is inaccessible from outside
  the module. Here inaccessible means both that the thing cannot be
  named (as today) but also additional guarantees in the case of a type:
  • For a type, it means that a value of that type cannot be directly
    accessed except as part of a trait object.

I think this is a very pretty useful guarantee! When you think of
security audits as the driving use case, you can also see why I want
it to be obvious whether a type is directly exposed outside the module
or not ("Do you see the word pub?").

[sfackler]

@nikomatsakis how would that rule interact with pub use?

[nikomatsakis]

On Fri, Aug 15, 2014 at 12:40:14PM -0700, Steven Fackler wrote:

@nikomatsakis how would that rule interact with pub use?

I think I spelled it out, but basically a pub use foo = bar requires
that bar is declared pub and reachable.

[glaebhoerl]

@nikomatsakis I can see how that guarantee would also be valuable. Wouldn't it make sense to extend it to encompass private modules as well, i.e. to be able to know for sure that anything in a private submodule is not leaked outside the parent module? That feels more "compositional" to me.

There's basically two changes the amendment proposes:

1. Make it illegal to expose an item with pub use that was not originally pub.
2. Consider an item public even if it has private components in its path (i.e. pub item in private submod).

Are these separable? Could we do 1. without 2.? That would provide all of the discussed guarantees. Are there any interesting/valuable module structures which would be ruled out?

[nikomatsakis]

On Sun, Aug 17, 2014 at 11:42:45AM -0700, Gábor Lehel wrote:

@nikomatsakis I can see how that guarantee would also be valuable. Wouldn't it make sense to extend it to encompass private modules as well, i.e. to be able to know for sure that anything in a private submodule is not leaked outside the parent module? That feels more "compositional" to me.

I've been thinking about this off and on. You are addressing what was
my biggest concern -- specifically that privacy can be a layered thing
and our design doesn't address that -- however I still feel
uncomfortable with a design that effectively "scans" for leaks. It
feels indirect and rather complicated. Basically what's happening is
that we are inferring "how public" an item is based on the pub use
declarations and so forth, rather than letting the user tell us. I
think I'd rather extend the language (perhaps at a late date) with an
official way to differentiate "public to anyone who can reach" and
"public to my parent and cousins".

There's basically two changes the amendment proposes:

1. Make it illegal to expose an item with pub use that was not originally pub.
2. Consider an item public even if it has private components in its path (i.e. pub item in private submod).

Are these separable? Could we do 1. without 2.? That would provide all of the discussed guarantees. Are there any interesting/valuable module structures which would be ruled out?

Yes, they are separable, but as I wrote above I am still skeptical
about having such a complicated check here. I'll ponder it a bit more,
but it feels like a kind of "design smell" (much as you suggested that
the existing use cases for exposing private items in public APIs
suggest missing features we might want to consider adding in the
future).

[glaebhoerl]

I think I'd rather extend the language (perhaps at a late date) with an official way to differentiate "public to anyone who can reach" and "public to my parent and cousins".

I'm not sure that this makes sense. I believe a module should be thought of as a self-contained unit, and that decisions about what to do with the items it exposes, including whether to expose them further, are the right and responsibility of the parent module, not the module itself. The way to say that the items in a mod should be "public to anyone who can reach", instead of "public to my parent and cousins", is pub mod.

I have a set of examples which I think should clearly delineate the things which should be rejected, as well as how to modify them to make them legal, and I believe that this is 'complete' and logically extends to cover all relevant cases in a way that maintains the desired guarantees. But I'm not yet sure of the best way to put the rules into words (and code).

Here they are. I've omitted the obvious cases where everything is in the same module.

Not OK:

mod m {
    pub struct S;
}
pub fn f() -> m::S;

OK:

mod m {
    pub struct S;
}
pub use m::S;
pub fn f() -> S;

Not OK:

struct S; 
pub mod m { 
    pub fn f() -> super::S; 
}

OK:

pub struct S; 
pub mod m { 
    pub fn f() -> super::S;
}

Not OK:

struct S; 
mod m { 
    pub fn f() -> super::S; 
} 
pub use f = m::f;

OK:

pub struct S; 
mod m { 
    pub fn f() -> super::S; 
} 
pub use f = m::f;

In particular pub use should be treated specially, because it is not referring to an item, but exposing it. So the item it exposes must have been declared pub, but, unlike "normal" items, it should be allowed to "reach into" private modules.

[ben0x539]

so this pr turned an accepted rfc around, basically?

[aturon]

@ben0x539 I'm not sure why the link wasn't included when this was merged, but this was discussed at a weekly meeting as with any RFC.

[ben0x539]

yeah, ok, but... at this point you might as well have retroactively rejected the old rfc, the current version shares neither the motivation nor the design with it and the changes don't really stem from discussion on the old rfc pr.

[SimonSapin]

Sorry I’m late to the party, but I don’t understand the motivation for this change. I find the previous reachability-based behavior to be superior, and don’t see a reason to prefer the current one. rust-lang/rust#30905 shows how the current behavior has both false positives and false negatives.

[nikomatsakis]

@SimonSapin

The reason that I, at least, prefer the current setup is that I only have to look at the definition of an item to understand how widely it can be referenced. In particular, if it is private, then I know that it does not leak outside the current module (whereas, without any rules at all, I have to scan the signatures of public items to know that for sure).

In the reachability-based variants, I have to scan and understand the pub use setups to determine the same information. We found experimentally that this was difficult: frankly, it was just hard to tell if the compiler was giving legit errors or not. Partly this was a poor compiler implementation: it should have given us a witness path, for example. But even if we improved the error message, it'd still be the case that I cannot easily deduce the exposure of an item.

That said, I agree that rust-lang/rust#30905 represents a real problem. I just disagree that reachability is the better fix. I personally would prefer a system like #1422, which allows us to directly specify publicity at a finer granularity. This way, your example from rust-lang/rust#30905 (actually, a slightly modified version):

pub mod outer {
    struct HostInternal;
    mod parser {
        use super::HostInternal;
        pub fn parse() -> HostInternal {...}
    }
}

could be accommodated by modifying pub fn parse to be pub(super) fn parse. Now, when I look at the definition of parse, I can see immediately that parse is something private to outer. In the reachability based variant, I would have to check for pub use and so forth to determine that.

Historically, we settled on pub and priv as the sole modifiers because we thought that you could model everything else with pub use. Clearly this is true. But I've also found it to be problematic in practice, since it makes the privacy structure kind of hard to discern -- in unsafe code in particular, that can be a real problem!

So perhaps the question then becomes whether you would be happy with the system proposed in #1422?

[petrochenkov]

By the way, there was some talks around 1.0 about keeping type ascribed source code on crates.io to avoid breakage from type inference changes.
The first version of this RFC ensured, that such type ascription would never lead to privacy errors, while the new version doesn't give such guarantee.