Private struct returned by public fn

[dodomorandi]

This discussion in the user forum highlighted an unexpected situation, in which a library exports a fn returning a private struct.

A simple test case is the following:

mod parent {
    mod private {
        pub fn test() -> Test {
            Test
        }
        
        #[derive(Debug)]
        pub struct Test;
    }
    
    pub use self::private::test;
}

use self::parent::test;

fn main() {
   println!("{:?}", test());
   // The following does not compile
   println!("{:?}", self::parent::private::Test);
}

@vitalyd said that it is not the first time this problem occurs, and that maybe there is an issue around.

I see this as a problem, even if minor, of the module infrastructure. Using the 2018 edition does not change the behaviour.

Do you think that this can be considered a bug as well? Should be fixed before Rust 2018? In the end, even if the current behaviour is wrong, fixing it is a breaking change.

EDIT: @vitalyd found the issue in tokio

[eddyb]

I don’t see any private items, other than parent and private, neither of which affects the privacy of test and Test.

[dodomorandi]

Test is accessible from parent, but private from outside because private is not pub. Therefore Test is not accessible from main, even if test is accessible and can return a Test.

Try that on playground

[eddyb]

“Not accessible” is correct, yes, but its also completely irrelevant for whether we allow using the type at all or not.

It was declared public, which is the only thing that matters.

This was discussed and settled years ago, I believe. @petrochenkov would be able to offer more specific links to RFCs etc.

EDIT: note that pub means “public to the universe”. You might want/be thinking of pub(crate) (public only inside the current crate) and/or pub(super) (public only inside the parent module).
The parent module being private does not downgrade pub children to pub(super).

[dodomorandi]

I see your point, but on the other hand this behaviour is error prone for crate authors.

I think that it is easy to re-export internal functions and forgetting to do the same for some kind of struct. Maybe it is incorrect to consider this something unwanted, but at the same time it is also difficult to help a developer who doesn’t want this situation.

To me, it is difficult to think about a type that I can get as output but I cannot even mention because “it’s private”. IMHO, if a struct is private, it shall not be exposed in any way.

The parent module being private does not downgrade pub children to pub(super) .

But the fact that pub use allows to expose to super module a function returning structs that are not accessible from super is some sort of privateness inconsistency IMO.

[eddyb]

It’s what we decided on. I’m not saying it’s not a tradeoff, but it’s not a bug.

I believe what you want is lints about exposing unreachable/unexported types/traits in reachable/exported APIs.
IIRC these were discussed before, I’m not sure why they haven’t happened yet.

[dodomorandi]

I’m not saying it’s not a tradeoff, but it’s not a bug .

Ok, you are absolutely right

On the other hand, I am having troubles finding a valid situation in which an unreachable (I liked the term) item should be usable. If it is not exported, it is probably designed to be not used at all from outside its mod. On the other hand, an user does not have any reason to doubt about the usability of the value returned from a function.

Even if this cannot be considered a bug, the term tradeoff implies that there are cases in which this situation is intended.

From what you said you have the experience to give me some nice counter-examples, and honestly I would like to be convinced that there are situations in which this behaviour is wanted from a crate developer.

[eddyb]

I can tell you one side of the tradeoff: it’s significantly easier to implement a correct system that checks for privacy the way it is defined today, compared to a system that’s based on reachability. It’s also more predictable, in some aspects.

[dodomorandi]

Do you think that it would be complex to implement a check on pub use-s? Because it looks like that this is the only way of creating these kind of inconsistencies in reachability. Correct me if there are other cases in which it is possible to do the same in other ways.

A basic idea could be to check that something that is pub use is not related to elements that are not reachable from that point. In theory, the compiler should be already be able to assert this situation, because it is able to check if something is private respect to a context. Am I wrong?

[gbutler]

I can tell you one side of the tradeoff: it’s significantly easier to implement a correct system

I’m probably wrong, but, it feels like there is the potential for a soundness hole with this sort of exposing of something internal to a private module by way of it being a return value of a re-exported function from within that module. It seems like this might open up a possibility of things external to the module being able to effect some invariant that unsafe code inside the module depends upon. I can’t think of a specific example right now, but, I’d be concerned about it. I’ll have to ponder on it when I have more time to see if I can come up with an example that demonstrates such a soundness hole.

That being said, it just feels wrong because “unsafe” code is supposed to be be able to count on certain invariants internal to a module for soundness and if you start to expose internals of a module, it seems like that is setting things up for failure.

More than likely, I’m over-thinking it though.

[eddyb]

The type was declared as public. It’s a bug in the unsafe code if it declared as pub types that were meant to remain internal details.

You can’t get this without writing pub which means “everyone, even outside this crate, gets to use this type, if they can get their hands on it”.

[gbutler]

It’s a bug in the unsafe code if it declared as pub

OK, yeah, I think I can agree with that. It just feels somehow wrong, but, I definitely see your point.

[petrochenkov]

@dodomorandi
The relevant RFC is https://github.com/rust-lang/rfcs/pull/2145, it includes the lint you are talking about as well.
It’s not yet implemented though (the tracking issue is https://github.com/rust-lang/rust/issues/48054), mostly due to 2018-edition-related work being more prioritized.

[dodomorandi]

Thank you!

If I am correct, what I was talking about is the unnameable_types lint, right? You explicitly said that

The “Voldemort type” (or, more often, “Voldemort trait”) pattern has legitimate uses

Can you give me a hint about a possible legitimate use? I am sorry, but I am unable to think about one of them

[repax]

Maybe one day declaring something like this will be possible.

pub use self::private::test as test() -> impl Debug;

Here test() is constrained to impl Debug for the outside world, but still fully transparent within parent.

[CAD97]

#[warn(unreachable_pub)] exists today. (Or at least it did the last time I checked.) It’s not perfect – cascading pub use don’t mark something as reachable – but it catches this case and is a good lint to turn on so long as you’re ok allowing false positives.

But this ability to create unnameable types is useful.

You can create a sealed trait hiearchy by making it impossible for users of a crate to implement a type. This is either done via a Sealed super trait which is only namable in your crate (thus unable to be implemented outside) or having a function that takes an Unnamable, thus preventing the trait from being implemented outside of your crate as they can’t name the types required to write the impl. (You can also just hide the trait itself if it’s only used for input polymorphism and the user doesn’t need to use it at all.)

[matt1985]

One use for voldemort traits is the Sealed trait pattern found in the Rust api guidelines

[dodomorandi]

Thanks @matt1985 and @CAD97, that is exactly what I was looking for… for traits

Does a similar application exists for structs? In that case, I am completely convinced that it have to be a warning lint and not an error.

[CAD97]

I believe that for structs everything can be handled by privacy directly (as you can mix pub and private members) rather than having to use an unnameable type.

You could use it in an enum variant to make it unconstructable, but then you’re pretty much better off just using a newtype variant around a struct for proper privacy control.

Unnameable types serve a similar propose to the never type, !. It’s just instead of a value that cannot be created without UB, you have a type that cannot be named, thus statically removing the ability for users to use some type instead of to call some function.

And unlike the never type, every crate does have to create their own unnameable type.

(That said, it already is an (imperfect) allow-by-default lint, #![warn(unreachable_pub)].)

[Ixrec]

Unnameable types serve a similar propose to the never type, ! . It’s just instead of a value that cannot be created without UB, you have a type that cannot be named, thus statically removing the ability for users to use some type instead of to call some function.

As for why anyone would want a type that users can get from a function but not construct themselves, the simplest example I know of is the “proof of work token” to force users of your API to call your functions in the right order.

fn step1() -> Step1Token;             // malloc, fopen, glCreateContext, connect
fn step2(Step1Token) -> Step2Token;   // write, read, mutate, draw, send
fn step3(Step2Token);                 // free, close, destroy

(taken from https://gankro.github.io/blah/linear-rust/#cant-be-used-more-than-once)