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

impl trait in type aliases does not trigger "private type in public interface" errors #63169

Closed
Nemo157 opened this issue Jul 31, 2019 · 8 comments
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. A-resolve Area: Name resolution A-visibility Area: Visibility / privacy F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-needs-decision Issue: In need of a decision. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Nemo157
Copy link
Member

Nemo157 commented Jul 31, 2019

The current implementation of existential_type does not trigger any "private type in public interface" errors, for the following code (playground):

#![feature(existential_type)]

existential type Foo: std::fmt::Debug;

pub fn foo() -> Foo {
    5usize
}

I would expect it to give an error like

error[E0446]: private type alias `Foo` in public interface
  --> src/lib.rs:8:1
   |
5  |   existential type Foo: std::fmt::Debug
   |   - `Foo` declared as private
...
8  | / pub fn foo() -> Foo {
9  | |     5usize
10 | | }
   | |_^ can't leak private type alias

(although, because normal type aliases are transparent to the visibility checking, I'm not certain exactly what the error message should say; it just seems bad to introduce another way to accidentally forget to make part of your API publicly nameable)

cc #63063

@Nemo157
Copy link
Member Author

Nemo157 commented Jul 31, 2019

@rustbot modify labels to A-impl-trait, C-bug, F-impl_trait_type_aliases, T-lang

@rustbot rustbot added A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. C-bug Category: This is a bug. F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 31, 2019
@Centril Centril added A-resolve Area: Name resolution A-visibility Area: Visibility / privacy labels Jul 31, 2019
@Centril
Copy link
Contributor

Centril commented Jul 31, 2019

cc @petrochenkov

@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jul 31, 2019
@petrochenkov
Copy link
Contributor

This is intentional, impl Trait is opaque and equivalent to dyn Trait from the privacy checking point of view.
For other crates the return type of foo looks like public std::fmt::Debug and doesn't leak any info specific to the private Bar.
(Except for Send/Sync which were explicitly decided to be leaked by impl Trait, unfortunately or not).

A motivating example:

pub fn f() -> impl Fn() { // OK
    || {} // It's hard to come up with something more private than a nameless fn-local closure.
}

RFC: https://github.com/rust-lang/rfcs/blob/master/text/2145-type-privacy.md

@Nemo157
Copy link
Member Author

Nemo157 commented Jul 31, 2019

The problem is that with RFC 2515 the impl Trait alias essentially becomes a new nominal type, by having a public API mentioning this unreachable alias you create APIs that cannot have their values stored and retrieved from non-inferred locations.

As an extended example this playground takes the above API and puts it into a local module (foo). A user of that module then attempts to use their own local impl Trait alias (MyFoo) to be able to store an instance of foo::Foo in a struct (since foo::Foo is private and cannot be named), that creates a new opaque type that doesn't unify with foo::Foo and so cannot be used with foo::bar.

@Nemo157
Copy link
Member Author

Nemo157 commented Jul 31, 2019

(btw Bar wasn't really anything to do with the bug, just leftover from me trying stuff out in that playground, I've removed it from the example).

@Centril
Copy link
Contributor

Centril commented Jul 31, 2019

The problem is that with RFC 2515 the impl Trait alias essentially becomes a new nominal type, by having a public API mentioning this unreachable alias you create APIs that cannot have their values stored and retrieved from non-inferred locations.

I'm not sure that's so much of a problem; notably you can still use foo::bar(foo::foo()); and inferred locals are still available if you need to store in a local.

What's nice about the current semantics is that type Alias = TypeExpr; will not need a special rule for Visibility X WasThereImplTrait which should simplify both the mental model and the compiler.

@Centril Centril added T-lang Relevant to the language team, which will review and decide on the PR/issue. I-needs-decision Issue: In need of a decision. and removed C-bug Category: This is a bug. labels Jul 31, 2019
@petrochenkov
Copy link
Contributor

@Nemo157
I see the issue, but type privacy is supposed to solve the opposite problem - preventing leaks, rather than ensuring that everything reachable can be named as well.
One of the lints proposed by RFC 2145 will help to detect unnameable things, but it's not implemented yet.

@Centril Centril added the requires-nightly This issue requires a nightly compiler in some way. label Aug 1, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Feb 18, 2021

I'm going to go ahead and close this as per the analysis above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impl-trait Area: `impl Trait`. Universally / existentially quantified anonymous types with static dispatch. A-resolve Area: Name resolution A-visibility Area: Visibility / privacy F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` I-needs-decision Issue: In need of a decision. requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Development

No branches or pull requests

5 participants