Skip to content

privacy exposure bug when mixing explicit type ascription and an impl on struct #10548

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

Closed
pnkfelix opened this issue Nov 18, 2013 · 5 comments
Closed
Labels
A-visibility Area: Visibility / privacy

Comments

@pnkfelix
Copy link
Member

Surely adding a type ascription shouldn't cause a privacy violation, right? And yet...

Example file (a.rs):

mod a {
    struct S;

    #[cfg(not(hide_impl))]
    impl S { }

    pub struct Expose;
    impl Expose { pub fn expose(&self) -> S { S } }
}

fn f_infer() {
    let _s;
    _s = a::Expose.expose();
}

#[cfg(demo_explicit)]
fn f_typed() {
    let _s : a::S;
    _s = a::Expose.expose();
}

fn main() {}

Some runs illustrating the bug. Note that the only difference introduced by hide_impl is whether or not the empty impl item for S is present, and therefore toggling it should not (IMO) be causing changes to the privacy semantics.

% rustc a.rs
% rustc --cfg demo_explicit a.rs
% rustc --cfg hide_impl a.rs
% rustc --cfg hide_impl --cfg demo_explicit a.rs
a.rs:18:13: 18:17 error: type `S` is private
a.rs:18     let _s : a::S;
                     ^~~~
error: aborting due to previous error
task 'rustc' failed at 'explicit failure', /Users/fklock/Dev/Mozilla/rust.git/src/libsyntax/diagnostic.rs:101
task '<main>' failed at 'explicit failure', /Users/fklock/Dev/Mozilla/rust.git/src/librustc/lib.rs:396

(Possibly related to #10545, but do note that the properties of these two scenarios do differ. In particular, the presence/absence of fields in the struct seems to be irrelevant for this bug.)

@alexcrichton
Copy link
Member

If anything is the bug here, it's that we're letting a return value be a private struct. It's already a little weird that we allow that, and it may be too unreasonable to disallow that. I believe that the privacy checker is correct in disallowing usage of a::S as a type (or a value as well), and I personally think that it's fine to return a private struct, you're just understanding that it's quite limiting on callers (because they can't really do a whole lot with it.

@pnkfelix
Copy link
Member Author

@alexcrichton yeah, the only use case I can think of for a pub function returning a private struct would be for chaining it together with another pub function that consumes that struct.

But its still weird to mark the type ascription as disallowed. Could it make more sense to disallow binding the intermediate value itself? (Basically forcing the client code to perform such chaining?) I'm really just thinking out loud right now, not sure if I actually would like having to work with such a rule.

(Having said that, I'm tempted to just say that we should just force any struct used for argument or return type on a pub fn to also be pub.)

@pcwalton
Copy link
Contributor

This seems like the correct behavior to me. Privacy is about the ability to name things in Rust; adding a name causes privacy to be checked.

@alexcrichton
Copy link
Member

This is a bug in privacy actually (a little confusing from the description), but this program should not compile (and it does)

mod a {
    struct S;
    impl S { }
}

fn foo(_: a::S) {
}

fn main() {}

This program should not be able to name a::S. That being said, I believe that this is the only bug that this issue should be referencing.

@alexcrichton
Copy link
Member

Closing as a dupe of #10545, my test case is essentially what's going on in that bug, and otherwise I believe everything is working as intended.

flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 6, 2023
…zation-nursery, r=flip1995

Move unnecessary_struct_initialization to nursery

changelog: none, assuming it makes into the same release as rust-lang#10489

Mostly because of rust-lang#10547 but there is also rust-lang#10548
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-visibility Area: Visibility / privacy
Projects
None yet
Development

No branches or pull requests

3 participants