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

Rules governing references to private types in public APIs not enforced in impls #28325

Closed
nikomatsakis opened this Issue Sep 9, 2015 · 6 comments

Comments

Projects
None yet
7 participants
@nikomatsakis
Contributor

nikomatsakis commented Sep 9, 2015

This code compiles, it should not:

mod x {
pub struct Foo { x: u32 }

struct Bar { x: u32 }

impl Foo {
    pub fn foo(&self, x: Self, y: Bar) { }
}
}

fn main() { }

cc @nrc

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Sep 9, 2015

Contributor

nominating, as this seems like a backcompat hazard.

Contributor

nikomatsakis commented Sep 9, 2015

nominating, as this seems like a backcompat hazard.

@bluss

This comment has been minimized.

Show comment
Hide comment
@bluss

bluss Sep 9, 2015

Contributor

See also #18082

Contributor

bluss commented Sep 9, 2015

See also #18082

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Sep 17, 2015

Contributor

triage: P-high

Contributor

nikomatsakis commented Sep 17, 2015

triage: P-high

@pnkfelix

This comment has been minimized.

Show comment
Hide comment
@pnkfelix

pnkfelix Sep 17, 2015

Member

(a slightly more complicated example that shows the privacy hole being exploited:

mod x {
    pub struct Foo { pub x: u32 }

    struct Bar { _x: u32 }

    impl Foo {
        pub fn foo(&self, _x: Self, _y: Bar) { }
        pub fn bar(&self) -> Bar { Bar { _x: self.x } }
    }
}

pub fn main() {
    let f = x::Foo { x: 4 };
    let b = f.bar();
    f.foo(x::Foo { x: 5 }, b);

}

)

Member

pnkfelix commented Sep 17, 2015

(a slightly more complicated example that shows the privacy hole being exploited:

mod x {
    pub struct Foo { pub x: u32 }

    struct Bar { _x: u32 }

    impl Foo {
        pub fn foo(&self, _x: Self, _y: Bar) { }
        pub fn bar(&self) -> Bar { Bar { _x: self.x } }
    }
}

pub fn main() {
    let f = x::Foo { x: 4 };
    let b = f.bar();
    f.foo(x::Foo { x: 5 }, b);

}

)

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Nov 12, 2015

Contributor

@petrochenkov do you think your rewritten privacy pass will fix this?

Contributor

nikomatsakis commented Nov 12, 2015

@petrochenkov do you think your rewritten privacy pass will fix this?

@petrochenkov

This comment has been minimized.

Show comment
Hide comment
@petrochenkov

petrochenkov Nov 12, 2015

Contributor

Yes, it should.
(But this part isn't written yet)

Contributor

petrochenkov commented Nov 12, 2015

Yes, it should.
(But this part isn't written yet)

bors added a commit that referenced this issue Dec 18, 2015

Auto merge of #29973 - petrochenkov:privinpub, r=nikomatsakis
Some notes:
This patch enforces the rules from [RFC 136](https://github.com/rust-lang/rfcs/blob/master/text/0136-no-privates-in-public.md) and makes "private in public" a module-level concept and not crate-level. Only `pub` annotations are used by the new algorithm, crate-level exported node set produced by `EmbargoVisitor` is not used. The error messages are tweaked accordingly and don't use the word "exported" to avoid confusing people (#29668).

The old algorithm tried to be extra smart with impls, but it mostly led to unpredictable behavior and bugs like #28325.
The new algorithm tries to be as simple as possible - an impl is considered public iff its type is public and its trait is public (if presents).
A type or trait is considered public if all its components are public, [complications](https://internals.rust-lang.org/t/limits-of-type-inference-smartness/2919) with private types leaking to other crates/modules through trait impls and type inference are deliberately ignored so far.

The new algorithm is not recursive and uses the nice new facility `Crate::visit_all_items`!

Obsolete pre-1.0 feature `visible_private_types` is removed.

This is a [breaking-change].
The two main vectors of breakage are type aliases (#28450) and impls (#28325).
I need some statistics from a crater run (cc @alexcrichton) to decide on the breakage mitigation strategy.
UPDATE: All the new errors are reported as warnings controlled by a lint `private_in_public` and lint group `future_incompatible`, but the intent is to make them hard errors eventually.

Closes #28325
Closes #28450
Closes #29524
Closes #29627
Closes #29668
Closes #30055

r? @nikomatsakis

@bors bors closed this in #29973 Dec 18, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment