Leaking private structs in public functions #85883

conradludgate · 2021-06-01T06:56:20Z

I tried this code:

use foo::bbb;

fn main() {
    let bar = bbb(); // cannot specify any type signature for `bar`
}

mod foo {
    use bar::Bar;

    pub fn bbb() -> Bar {
        Bar
    }

    mod bar {
        pub struct Bar;
    }
}

I expected to see this happen: Rust complain about the leaking of private structs.

Instead, this happened: It compiles, this has caused it to creep into some crates. For instance, this is in tonic 0.4.3

Fixing this would likely be a harsh breaking change (as I mentioned, it's already in at least 1 crate, possibly many more). So it would probably have to live across an edition change. It's possibly too late to get a fix in for Rust 2021.

For now, I've suggested it as a possible lint in clippy

aticu · 2021-06-01T09:05:06Z

The type Bar is marked with pub, so it's public. No private type is leaked. If you remove the pub or change it to pub(super), then the compiler will complain.

What you're probably referring to is that the type Bar cannot be named outside of the foo module. It can still be used normally in every other way though.
As far as I know this is intended behavior. It is used on occasion to implement something known as sealed traits.

conradludgate · 2021-06-01T09:30:21Z

Yes, it is technically public, but in all practicality, it's private. As you say, if you change the visibility, it does indeed error because it would be bad to return a type that cannot be annotated by the caller, yet this is exactly what the code above does. So this should also be an error

I am aware it's an intended feature for traits, but not for structs.

conradludgate · 2021-06-01T09:33:58Z

Specifically, the error is "A private item was used outside its scope.". Does that description not fit this scenario? Bar, in the perspective of the main function, is private

aticu · 2021-06-01T10:17:24Z

The following code compiles:

use foo::bbb;

fn main() {
    let bar = bbb(); // cannot specify any type signature for `bar`
    println!("{}", bar.some_inner_field);
    bar.some_method();
}

mod foo {
    use bar::Bar;

    pub fn bbb() -> Bar {
        Bar {
            some_inner_field: 42
        }
    }

    mod bar {
        pub struct Bar {
            pub some_inner_field: u8
        }
        
        impl Bar {
            pub fn some_method(&self) {
                println!("called a public method of an unnameable type");
            }
        }
    }
}

Bar can be used like ever other public type, except that it's impossible to name it.
As far as I understand it the type and the name of the type are two different "things", the type can be public without the name being public too.

If this were changed for structs, it would make the language less consistent, since it is intended for traits (and maybe even for structs too, though I don't know of any use cases).
Is there a specific problem you're trying to avoid with this error or why are you asking to have this code produce an error?

Maybe someone with more experience on actual use cases for this or how it came to be could weigh in too.

conradludgate · 2021-06-01T10:28:46Z

The reason I consider this a problem is because of hyperium/tonic#672 - notably https://docs.rs/tonic/0.4.3/tonic/transport/server/struct.Router.html#method.add_service this function returns Router<S, Or<A, B, Request<Body>>>, I'm trying to wrap the functionality of tonic, but I cannot do so as current because I cannot name that type in my wrapper code. I do personally think that any type that I can own, I should be able to write it's type down completely.

I have submitted a PR to tonic to get the visibility changed, but I do feel strongly that Rust (or clippy) should flag it. If you do want to return a type but have it private, I think it should be behind a trait and with impl Trait being the returned type.

nagisa · 2021-06-01T22:25:23Z

This is (ability to use un-nameable, but public types in a public API) a conscious design choice, I believe. There are valid use-cases for structs, too. For instance when used as an argument in a trait method, it can prevent implementors providing implementation.

There's perhaps slightly less value in doing so for return types, however, especially as it should be possible to name the return type via the associated Output type of the FnOnce trait, regardless, at least in some instances.

conradludgate · 2021-06-02T06:40:08Z

Once again, it's a feature for traits. But I see absolutely no use for a returned struct to be un-nameable.

I think if you want to have that power, you can create a trait (with a similar technique to seal it), implement the trait for your private struct, then return impl Trait

At least that way, I can still name something without needing the explicit type. The tonic example I gave above does not use this method, and it makes it impossible to build a wrapper around it.

Boddlnagg · 2021-06-02T14:39:56Z

Trying to use some unstable features to come up with a way to name the "private" type I ended up with an ICE:

#![feature(unboxed_closures)]
#![feature(min_type_alias_impl_trait)]
#![feature(impl_trait_in_bindings)]

use foo::bbb;

type Fun = impl Fn<()>;
static BBB: Fun = bbb;
type Out = <Fun as FnOnce<()>>::Output;
    

fn main() {
    println!("{}", std::any::type_name::<Fun>());
    println!("{}", std::any::type_name::<Out>());
    
    let bar: Out = BBB();
}


mod foo {
    use bar::Bar;

    pub fn bbb() -> Bar {
        Bar
    }

    mod bar {
        pub struct Bar;
    }
}

aticu · 2021-06-02T15:27:08Z

I'm trying to wrap the functionality of tonic, but I cannot do so as current because I cannot name that type in my wrapper code. I do personally think that any type that I can own, I should be able to write it's type down completely.

I do agree that this sort of situation is something that should be prevented from happening accidentally.
A clippy lint for that sort of thing is probably a good place to start.
However since there do seem to be valid use cases for a similar situation, I don't think that it can be considered a bug.

Trying to use some unstable features to come up with a way to name the "private" type I ended up with an ICE:

This ICE is unrelated to the issue at hand. I've tried to reduce it and managed to come up with this:

#![feature(unboxed_closures)]
#![feature(min_type_alias_impl_trait)]
#![feature(impl_trait_in_bindings)]

type Fun = impl Fn<()>;
static BBB: Fun = bbb;
type Out = <Fun as FnOnce<()>>::Output;

fn bbb() {}

fn main() {
    let bar: Out = BBB();
}

I was trying to look around for an issue with a similar ICE and I think this might be #85113.

inquisitivecrystal · 2021-06-05T01:05:12Z

@rustbot label +A-visibility +T-compiler +T-lang

mbartlett21 · 2021-06-10T14:15:57Z

This works to name the type (if it is 'static):

#![feature(min_type_alias_impl_trait)]
#![feature(impl_trait_in_bindings)]

type Out = impl core::any::Any;
fn main() {
    println!("{}", core::any::type_name::<Out>());
    let _v: Out = foo::bbb();
}

mod foo {
    use bar::Bar;
    pub fn bbb() -> Bar { Bar }
    mod bar {
        pub struct Bar;
    }
}

(playground)

ramosbugs · 2021-07-23T00:23:07Z

agree with OP on this issue for return types. the same thing shows up when re-exporting traits via pub use but forgetting to also re-export the return types. neither cargo check nor clippy complain in that case (see open-telemetry/opentelemetry-rust#600 for an example of how easy it is to make that mistake currently). another minimal repro: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=1f7891afdc5aca8b6ac1a88542cace5d

pub use inner::Foo;

mod inner {
    pub struct Bar;

    pub trait Foo {
        fn foo() -> Bar;
    }
}

elichai · 2022-01-06T09:40:18Z

Can we maybe add a warning for that? in rust-secp256k1 we accidentally exposed a function that accepts a semi private struct: https://docs.rs/secp256k1/0.21.0/secp256k1/struct.XOnlyPublicKey.html#method.tweak_add_check so it would be great if the compiler/clippy could've warn us

conradludgate added the C-bug Category: This is a bug. label Jun 1, 2021

aticu mentioned this issue Jun 2, 2021

ICE with opaque type #85113

Closed

rustbot added A-visibility Area: Visibility / privacy 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. labels Jun 5, 2021

aticu mentioned this issue Jun 10, 2021

ICE: broken MIR with impl_trait_in_bindings #86201

Closed

conradludgate mentioned this issue May 21, 2022

Better pattern argument naming in rustdoc #97048

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leaking private structs in public functions #85883

Leaking private structs in public functions #85883

conradludgate commented Jun 1, 2021

aticu commented Jun 1, 2021

conradludgate commented Jun 1, 2021

conradludgate commented Jun 1, 2021 •

edited

Loading

aticu commented Jun 1, 2021

conradludgate commented Jun 1, 2021

nagisa commented Jun 1, 2021 •

edited

Loading

conradludgate commented Jun 2, 2021 •

edited

Loading

Boddlnagg commented Jun 2, 2021

aticu commented Jun 2, 2021

inquisitivecrystal commented Jun 5, 2021

mbartlett21 commented Jun 10, 2021

ramosbugs commented Jul 23, 2021

elichai commented Jan 6, 2022

Leaking private structs in public functions #85883

Leaking private structs in public functions #85883

Comments

conradludgate commented Jun 1, 2021

aticu commented Jun 1, 2021

conradludgate commented Jun 1, 2021

conradludgate commented Jun 1, 2021 • edited Loading

aticu commented Jun 1, 2021

conradludgate commented Jun 1, 2021

nagisa commented Jun 1, 2021 • edited Loading

conradludgate commented Jun 2, 2021 • edited Loading

Boddlnagg commented Jun 2, 2021

aticu commented Jun 2, 2021

inquisitivecrystal commented Jun 5, 2021

mbartlett21 commented Jun 10, 2021

ramosbugs commented Jul 23, 2021

elichai commented Jan 6, 2022

conradludgate commented Jun 1, 2021 •

edited

Loading

nagisa commented Jun 1, 2021 •

edited

Loading

conradludgate commented Jun 2, 2021 •

edited

Loading