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

Semantic "private in public" enforcement. #1671

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions text/0000-semantic-private-in-public.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
- Feature Name: semantic_private_in_public
- Start Date: 2016-07-09
- RFC PR: (leave this empty)
- Rust Issue: (leave this empty)

# Summary
[summary]: #summary

Enforce that public APIs do not expose private definitions at the semantic level, while allowing the use of private aliases and blanket implementations for convenience and automation.

# Motivation
[motivation]: #motivation

The "private-in-public" rules ensure the transitivity of abstraction. That is, one must be able to name types and bounds used in public APIs, from anywhere else.
Copy link
Contributor

@petrochenkov petrochenkov Jul 9, 2016

Choose a reason for hiding this comment

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

"able to name" is not entirely correct, "able to name after addition of arbitrary number of reexports" is closer to the truth. E.g.

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

is allowed because S is potentially nameable outside of m (if reexported), but not actually nameable (private-in-public checker doesn't analyze reexport chains to determine actual nameability by design).
I'd like to avoid promising namebility in docs, including RFCs, because it's already one of the most common misconceptions about our privacy system.
EDIT: The definition of "less public" below also uses nameability ("can be referred to by any name").

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I remember reading about this before, but I wasn't clear on what decisions were taken.
It does weaken the "transitivity of abstraction" argument in that case, sadly.

Copy link
Contributor

Choose a reason for hiding this comment

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

How odd, link to rational for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks so much!

This property can be relied upon to create perfect proxies for generic types, functions and trait implementations.
However, the current set of rules is too strict and ignores any semantic equivalence:
```rust
type MyResult<T> = Result<T, String>;

#[derive(Clone)]
struct Wrap<T>(T);
Copy link
Contributor

@petrochenkov petrochenkov Jul 9, 2016

Choose a reason for hiding this comment

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

The basic idea behind the private-in-public checker is that values of private types (in addition to types themselves) can't leave their module.
Can a public function/method be tricked into returning a private type (with traits + associated types + type parameters + whatever) if it's used in one of its predicates like this PrivateType: Bounds?
Also, where can I read something about predicate elaboration?

Copy link
Member Author

Choose a reason for hiding this comment

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

The return type is in the signature and checked as part of that, regardless of what happens with bounds.
With this RFC you can still always write a perfect proxy for functions, methods, impls, etc.

"Elaboration" as used in this RFC is described in the detailed design section. I should probably be more specific about it because it only does the work necessary to rewrite private bounds in terms of more public ones.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an example of leaking a private type through bounds, the returned type T::Alias seems to be completely public if checked separately. I wonder if something like this can be constructed with PrivateWrap<T>: Bound-like bounds. (I fully support the goals of the RFC, just trying to figure out possible holes.)

pub trait Tr<T> {
    type Alias = T;
}
impl<T, U> Tr<T> for U {}

struct Priv;
pub fn f<T>() -> T::Alias
    where T: Tr<Priv>
{
    panic!()
}

Copy link
Member Author

Choose a reason for hiding this comment

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

After associated type resolution, the return type of f is Priv and that doesn't pass the check.
Even if it didn't resolve to any concrete type, the return type would be <T as Tr<Priv>>::Alias which still contains Priv so it doesn't pass the check either.

Copy link
Contributor

@petrochenkov petrochenkov Jul 9, 2016

Choose a reason for hiding this comment

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

By the way, if you do the T::Alias -> <T as Tr<Priv>>::Alias transformations before checking, then predicates can become completely unchecked, because nothing private can be leaked through them1.
This is more significant relaxation of the rules, but it is simpler and removes the need in the elaboration procedure.
We discussed this with @pnkfelix last year and both were generally in favor.

1"nothing private can be leaked" is the core guarantee from the current privacy system, as opposed to "nameability" from the old system.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, that does reduce this RFC to "check types after normalization", I wish that was better advertised as an option (it seems perhaps even simpler than the current implementation).


pub fn foo<T>(_: T) -> MyResult<()>
where Wrap<T>: Clone { Ok(()) }
```

The example above does not compile right now, because of `MyResult` and `Wrap` being private, even though a perfect proxy can be written as such:

```rust
fn bar<T: Clone>(x: T) -> Result<(), String> {
api::foo(x)
}
```

This limitation most notably prevents derive and similar macros from generating bounds on field types, as they may contain private types, although most of the time the bound can be written in terms of type parameters and public types (`T` below), or is not needed at all (`U` below):
```rust
#[derive(Clone)]
pub struct Foo<T, U>(Wrap<T>, Wrap<Rc<U>>);
```

Deriving cannot but add both a `T: Clone` and a `U: Clone` bound, in the current implementation, which is more restrictive than necessary, and ironically prevents automatic generation of perfect wrapper types.

# Detailed design
[design]: #detailed-design

Function signatures, public field types, types of statics, constants and associated constants, types assigned to type aliases and associated types, and where clauses (after elaboration) must not be *less public* than the item they are found in.

The previous definition of *less public* relied solely on paths as they appear in the source, but after this RFC, it is more fine-grained:

An item `X` is *less public* than another item `Y` if there exists a module from where `Y` can be referred to (by any name) whereas `X` can't, taking into account `pub(restricted)` and any other privacy semantics.

```rust
pub mod m {
struct A;
// A is less public than B
pub(crate) trait B {}
// B is less public than c
pub fn c() {}
}
```

A type or bound is *less public* than an item `X` if it refers to any type or trait definition that is *less public* than `X`, after resolving aliases and associated types.

Where clauses in an item `X` are elabored as follows:
* type aliases and associated types are resolved as with all types
* lifetime bounds *less public* than `X` are replaced with lifetime bounds on type and lifetime parameters
* for each trait bound *less public* than `X`:
* a list of applicable implementations is computed
* because the bound refers to items that cannot be exported, coherence will prevent applicable implementations from existing in downstream crates
* type parameters of `X` are assumed to match any type, regardless of what other bounds `X` has
* if there is exactly one applicable `impl`, the bound is replaced with the where clauses of that `impl`, after elaborating them as well

The set of bounds left after the recursive elaboration of `X`'s where clauses must not be *less public* than `X`, even if the original where clauses are allowed to.

Example for use in deriving, without restricting the user or exposing private details:
```rust
#[derive(Debug)]
struct Wrap<T>(T);

#[derive(Copy, Clone, Debug)]
pub struct Ref<'a, T: 'a>(&'a Wrap<T>);

// deriving will produce:
impl<'a, T> Copy for Ref<'a, T>
where &'a Wrap<T>: Copy {}
impl<'a, T> Clone for Ref<'a, T>
where &'a Wrap<T>: Clone {...}

impl<T> Debug for Wrap<T>
where T: Debug {...}
impl<'a, T> Debug for Ref<'a, T>
where &'a Wrap<T>: Debug {...}

// after elaborating where clauses:
impl<'a, T> Copy for Ref<'a, T> {}
impl<'a, T> Clone for Ref<'a, T> {...}

impl<T> Debug for Wrap<T>
where T: Debug {...}
impl<'a, T> Debug for Ref<'a, T>
where T: Debug {...}
```

# Drawbacks
[drawbacks]: #drawbacks

Browsing sources and generating documentation becomes more complex, as private details need to be replaced with equivalent public versions before using them from other modules/crates.

The "one applicable `impl`" rule works well with deriving, but adding a private `impl` can break adjacent public items, whereas the existing strategy of placing bounds on type parameters would continue to work, however restrictive it may be in general.

# Alternatives
[alternatives]: #alternatives

We could leave the current situation as-is, or just resolve type aliases and associated types, leaving deriving in the same suboptimal state.

# Unresolved questions
[unresolved]: #unresolved-questions

How much catering do we need to do to public re-exports out of private modules?

Is coherence guaranteed to prevent the existence of downstream trait implementations that match a bound using both type parameters and unexported type definitions?