-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Amend RFC 48 #200
Amend RFC 48 #200
Conversation
|
||
* The trait being implemented is public. | ||
* All input types (currently, the self type) of the impl are public. | ||
* *Motivation:* If any of the input types or the trait is public, it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be "are not public"?
The pub use barrier::*;
mod barrier
{
pub struct A;
struct B;
} |
|
||
Note that the path to the public item does not have to be private. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean "have to be public"?
The reason I didn't (and still don't) think "is the item declared
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. |
On Thu, Aug 14, 2014 at 06:51:37PM -0700, Gábor Lehel wrote:
I realize now that the threat model you are concerned about is I would like a guarantee that items declared as private are truly In contrast, I am not actually concerned about people inconsistently Basically the guarantee I want to achieve is:
I think this is a very pretty useful guarantee! When you think of |
@nikomatsakis how would that rule interact with |
On Fri, Aug 15, 2014 at 12:40:14PM -0700, Steven Fackler wrote:
I think I spelled it out, but basically a |
@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:
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? |
On Sun, Aug 17, 2014 at 11:42:45AM -0700, Gábor Lehel wrote:
I've been thinking about this off and on. You are addressing what was
Yes, they are separable, but as I wrote above I am still skeptical |
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 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:
OK:
Not OK:
OK:
Not OK:
OK:
In particular |
6357402
to
e0acdf4
Compare
so this pr turned an accepted rfc around, basically? |
@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. |
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. |
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. |
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 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 Historically, we settled on So perhaps the question then becomes whether you would be happy with the system proposed in #1422? |
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. |
Update documentation for stream::fuse.
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 andimpl
definitions are related.