Skip to content

Commit

Permalink
Start writing the nameable_interfaces RFC
Browse files Browse the repository at this point in the history
  • Loading branch information
jseyfried committed Feb 17, 2016
1 parent 1fea0c8 commit a23a5c9
Showing 1 changed file with 116 additions and 0 deletions.
116 changes: 116 additions & 0 deletions text/0000-nameable-interfaces.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
- Feature Name: nameable_interfaces
- Start Date: 2016-2-10
- RFC PR:
- Rust Issue:

# Summary
[summary]: #summary

This RFC requires that an item which is nameable from a given module have an interface
that is nameable from the module.

An item is "nameable" from a module if there is a path in the module's scope that resolves
to the item, possibly through a reexport.
By the "interface" of an item, I am referring to the "public-facing parts" of that item as defined
in [RFC #0136](https://github.com/rust-lang/rfcs/pull/0136),
i.e. the parts of the item that the RFC requires to be public whenever the item is public.

# Motivation
[motivation]: #motivation

The primary motivation for nameable interfaces is refactorability.
Nameable interfaces ensure that the type of any expression is nameable, guaranteeing that many

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Feb 17, 2016

Not completely, some additional checks are required to stop private types leaking through type inference (see rust-lang/rust#30476).
I also find the ability to type annotate expressions without fear of privacy violations a strong argument. Unfortunately, there are still types that are unnameable for reasons other than privacy - closures, function items. So you still can't produce completely type annotated source code (there were plans too keep such code on crates.io to avoid possible type-inference related breakage).

This comment has been minimized.

Copy link
@jseyfried

jseyfried Feb 18, 2016

Author Owner

Good point, I hadn't considered that.

common types of refactoring are always possible, including
- type annotating a let declaration,
- refactoring an expression without early returns into a function item,
- refactoring a closure without upvars into a function item, and
- refactoring a constant expression into a constant item.

Another motivation is documentability. This RFC would ensure that rustdoc never has to include
an inaccessible path in the documentation for a crate.

# Detailed design
[design]: #detailed-design

TODO describe design in more detail

Examples:
```rust
mod foo {
mod bar {
pub struct Bar;
}

pub fn f() -> bar::Bar {}
//~^ ERROR/WARN `f` is visible outside `foo`, but `bar::Bar` is not.
//~| NOTE consider either not declaring `f` with `pub` or reexporting `bar::Bar`.
}

mod foo {
mod bar {
pub struct Bar;
}

mod baz {
pub fn f() -> bar::Bar {}
//~^ ERROR/WARN `f` is visible outside `foo`, but `bar::Bar` is not.
}

pub use self::baz::f;
//~^ NOTE consider either not reexporting `f` or also reexporing `bar::Bar`.
}
```

# Drawbacks
[drawbacks]: #drawbacks

One drawback is the hassle for developers who have to fix unnameable interfaces or face warnings and
eventually breakage (unless we keep it a warning indefinitely).
One way to ameliorate this is to have clear warnings that suggest adding specific reexports
that would bring the code up to compliance.


The other main drawback is that adding the suggested reexports can sometimes expose previously
private implementation details. More specifically,
- reexporting a struct or an enum can allow access to its inherent methods,
- reexporting a struct can allow access to its constructor and fields,
- reexporting a trait allows access to its methods, and
- reexporting an enum allows access to its variants and their fields.

[RFC #1422](https://github.com/rust-lang/rfcs/pull/1422) will address the first two points by
allowing developers to rehide any exposed inherent methods, struct constructors, or struct fields.

More specifically, once can rehide inherent methods and struct fields by declaring them with the
pub(restricted) syntax to restrict their visibility to where the corresponding struct or enum was
nameable.
For structs with fields, rehiding the fields will also rehide the constructor.
For structs without fields, one must add a pub(restricted) dummy field to rehide the constructor.

To rehide enum variants and trait methods, RFC #1422 will have to be extended to allow the
pub(restricted) syntax on trait methods, enum variants, and the variants' fields. I think this
extension would be a good idea anyway.

# Alternatives
[alternatives]: #alternatives

We could choose not to require that interfaces be nameable.

# Unresolved questions
[unresolved]: #unresolved-questions

We could make nameable interfaces not only necessary but also sufficient to pass the
`private_in_public` check in `librustc_privacy`. This would allow, for example,
```rust
mod foo {
struct Bar; // `Bar` is nameable inside `foo`
mod baz {
pub fn f() -> Bar {} // Since f is not nameable outside `foo`, this use of `Bar` is ok.
}
}
```

This addition would make nameability synonymous with visibility, which seems to be what most people
expect (as evidenced by the discussions in rust issues
[#18082](https://github.com/rust-lang/rust/issues/18082),
[#23585](https://github.com/rust-lang/rust/issues/23585), and
[#30905](https://github.com/rust-lang/rust/issues/30905)).

3 comments on commit a23a5c9

@jseyfried
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a draft version of an RFC I would like to propose (rendered).

@petrochenkov I know you implemented the current public_in_private rules -- do you think this RFC is a good idea? Am I overlooking something important?

@petrochenkov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A historical note:
Private-in-public rules were based on nameability first (rust-lang#136).
Then some experience was gained with that rules and some core team members became allergic to it (see meeting minutes here and Niko Matsakis' summary here).
So, the new RFC (rust-lang#200) was written making the rules local and based exclusively on pub annotations.
After that Niko is happy, but users consider the rules counterintuitive and come up with both "false positive" and "false negative" examples.
The implementation was pretty buggy and this RFC was completely implemented only 1.5 years later, breaking people's code in the process. The most of broken code were actual private items in public interfaces and not
"false positives" like

mod foo {
    struct Bar; // `Bar` is nameable inside `foo`
    mod baz {
        pub fn f() -> Bar {} // Since f is not nameable outside `foo`, this use of `Bar` is ok.
    }
}

It turns out that people actually want to use private types and traits in their interfaces. We've been teaching that people to hide their "reachable" private types in private inner modules and mark them as pub since then. Prohibiting this trick now after promoting it for some time would look very strange.
Personally, I think that nameability constraints are useful and would make a good clippy lint, but the ship for making them language rules sailed, maybe in Rust 2.0...

pub(restricted) for trait items, inherent impls, fields, etc is a totally good idea.
There were some discussions about it before, you can read the recent one here, it also mentions sealed(restricted) for "implementation privacy" (as opposed to "use privacy").
Basically, pub(restricted) is our main tool for salvaging the good parts of the privacy system in a backward compatible way and making something better from them. For example, the problem mentioned above can be solved with pub(restricted) like this:

mod foo {
    struct Bar; // `Bar` is nameable inside `foo`
    mod baz {
        pub(super) fn f() -> Bar {} // Since f is not nameable outside `foo`, this use of `Bar` is ok.
    }
}

Bar can be used in the interface of f because its visibility is guaranteed to be larger-or-equal than visibility of f. This way users get rid of a false positive and Niko stay happy because the reasoning is still local.

@jseyfried
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, thanks for the detailed exposition and references!
I still think that the language would be better if nameability and reachability were unified, but you've convinced me that probably won't be feasible until 2.0 and that this draft may not be the best approach.

Please sign in to comment.