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

Refine the story of externally referenced private structures. #10573

Closed
alexcrichton opened this issue Nov 20, 2013 · 27 comments
Closed

Refine the story of externally referenced private structures. #10573

alexcrichton opened this issue Nov 20, 2013 · 27 comments
Labels
A-visibility Area: Visibility / privacy. P-low Low priority

Comments

@alexcrichton
Copy link
Member

From my understanding, the story of a private structure that is returned from a public api is a little hazy today. For example, what's the reasonable thing to do in these situations:

  1. Can I call a method on a private struct?
  2. Can I access fields of a private struct?
  3. Can I return a private struct?
  4. Can I reference a public type through which I can reach a private struct?
  5. Can I reference a private type which I receive through a public method in a type signature?

Questions 1-4 are answered with 'yes' today, but the answer to question 5 is no. I believe that these should either all be no, or all be yes. In my opinion, it's just too painful for them to all be 'no', and it's unclear to me whether there's benefit to that.

Nominating, we need to decide this.

@alexcrichton
Copy link
Member Author

cc #10548
cc #10545

@huonw
Copy link
Member

huonw commented Nov 20, 2013

Is there any use case for them to be "yes"? i.e. is there ever a situation were one actually wants/needs to return a struct that is completely private (rather than just the fields)? I would think the most common time it occurs is accidentally, if one doesn't happen to have a test outside the module in which the struct is defined.

(Of course, the complexity of making all-no work might be hard, e.g. pub struct Foo { x: Private } should warn/error but pub struct Foo { priv x: Private } shouldn't.)

@nikomatsakis
Copy link
Contributor

On Tue, Nov 19, 2013 at 05:10:43PM -0800, Alex Crichton wrote:

In my opinion, it's just too painful for them to all be 'no', and it's unclear to me whether there's benefit to that.

Why do you say it's "too painful"? Is this such a common scenario?

I don't have a strong opinion about this at the moment. It seems like
the inconsistent answers to your questions fall out from having
fine-grained privacy controls, and in particular from allowing public
things from within private ones. I could be easily persuaded that this
is not a useful thing, and that privacy rules and defaults ought to
prevent public fields in private structs and so on.

@alexcrichton
Copy link
Member Author

This has been currently uncovered as part of the runtime's implementation. Inside of std::rt there are things like the kill and select modules which are intended to be private, but std::task and std::select must use their implementations currently. This could just mean that the runtime needs to get reorganized though.

I'd imagine that @huonw's example of having a private structure in a public one might be common, but then again that's just conjecture.

@nikomatsakis
Copy link
Contributor

On Wed, Nov 20, 2013 at 10:47:58AM -0800, Alex Crichton wrote:

Inside of std::rt there are things like the kill and select
modules which are intended to be private, but std::task and
std::select must use their implementations currently.

This seems to be an independent problem. Essentially we've
"oversimplified" the privacy system down to pub/priv, meaning that to
encode ACLs that don't fit nicely into a tree, one must use modules
and public exports.

@sfackler
Copy link
Member

Having a private structure as a private field inside of a public one is pretty common in my experience.

@nikomatsakis
Copy link
Contributor

@sfackler right but it's the opposite that we're discussing here, I think? that is, a private struct name with public fields.

@pnkfelix
Copy link
Member

It is not clear what the implications are of sticking with our current rules. P-low.

@nikomatsakis
Copy link
Contributor

Perhaps related to http://www.researchgate.net/publication/221320580_Islands_Aliasing_Protection_in_Object-Oriented_Languages -- though I can't remember the details of what this paper involved :)

@pnkfelix
Copy link
Member

In particular, my main worries, that I mentioned during triage, are:

  1. someone experiencing surprise at e.g. not being able to explicitly annotate all of their let-bound variables with correct types (and instead the type-inferencer is the only entity with the authority to do the type assignment) -- a la privacy exposure bug when mixing explicit type ascription and an impl on struct #10548, and, orthogonally,
  2. someone misinterpreting what the implications of priv are, e.g. thinking "if I make a type priv, that means expressions with that type cannot possibly be passed back to any callers, at least not with such a specific type, (as opposed to e.g. being hidden behind an Any or an object type of some sort."

Having said that: If we impose some additional small constraints, like "you cannot access members from a type that is priv to you", then I will probably be satisfied.

@alexcrichton
Copy link
Member Author

We were thinking today that privacy is correct in denying access to the name of private types, but it may not be correct in denying access to the insides of private types. There are two remaining questions to me:

  • Should a private type have its public fields accessible?
  • Should a private type have its public methods callable?

Privacy currently says yes to both of these questions, but it's very easy for privacy to say no.

@huonw
Copy link
Member

huonw commented Nov 22, 2013

I'd personally say that it should just be illegal for private types to appear in such a way that means they're ever publically accessible, which would resolve both those questions, i.e.

mod foo {
    struct Private;

    pub struct Public { not_priv: Private } // not ok
    pub struct PrivField { priv not_not_priv: Private } // ok

    fn private() -> Private {} // ok

    pub fn public() -> Private {} // not ok

    pub fn closure(_: |&Private| -> int) {} // not ok
}

(I guess this would just involve checking that every type mentioned in a public function signature is accessible in the parent module.)

@nikomatsakis
Copy link
Contributor

We can also address the question another way:

  • Should a private type even be allowed to have public fields/methods?
  • Should a public field be allowed to have a private type?
  • Should a public method be allowed to return a private type?

Note though that even if you answer no to the above questions,
if the user did somehow get ahold of an instance of a private
type -- which may be impossible, not sure -- they'd be able to
invoke trait methods.

@glaebhoerl
Copy link
Contributor

Think I agree with @huonw. Just an outsider looking in, but it seems to me that the straightforward way to resolve this would be to go ahead and make the things illegal, and see if anything important breaks. It's easier to re-allow something later if there turns out to be a use case than the other way around.

The one case I can think of offhand where you might want to have a private thing showing up in public API is to have a private super-trait of a public trait. This has the effect that other modules can use the trait, put it in their type signatures, and so forth, everything except make an impl for it. (Then again that might be better expressed by having a #[no_external_impls] attribute or something, but right now there's no such thing as far as I know.)

@ktt3ja
Copy link
Contributor

ktt3ja commented Dec 22, 2013

I also agree with huonw. It seems a lot more straightforward to just remember that priv => those outside don't have to worry about it at all, and thus private items are solely implementation details of something.

c-a added a commit to c-a/rust that referenced this issue Jan 12, 2014
Since reader::vuint_at() returns a result of type reader::Res it makes sense
to make it public.

Due to rust's current behavior of externally referenced private structures,
rust-lang#10573, you could still use the result and
assign it to a variable if you let the compiler do the type assignment,
but you could not explicitly annotate a variable to hold a reader::Res.
@emberian
Copy link
Member

Another issue: can a public trait inherit from a private one?

@sfackler
Copy link
Member

@cmr I would think yes; it'd be an entirely private implementation detail. Code that can't see the private trait just can't use it, which seems consistent to me.

@huonw
Copy link
Member

huonw commented Jan 20, 2014

cc #11685 for an(other) example of a problem this would resolve (specifically, making it illegal to return private structures from public functions would make it impossible to compile that example).

huonw added a commit to huonw/rust that referenced this issue Feb 27, 2014
These are types that are in exported type signatures, but are not
exported themselves, e.g.

    struct Foo { ... }

    pub fn bar() -> Foo { ... }

will warn about the Foo.

Such types are not listed in documentation, and cannot be named outside
the crate in which they are declared, which is very user-unfriendly.

cc rust-lang#10573
huonw added a commit to huonw/rust that referenced this issue Feb 28, 2014
These are types that are in exported type signatures, but are not
exported themselves, e.g.

    struct Foo { ... }

    pub fn bar() -> Foo { ... }

will warn about the Foo.

Such types are not listed in documentation, and cannot be named outside
the crate in which they are declared, which is very user-unfriendly.

cc rust-lang#10573
bors added a commit that referenced this issue Feb 28, 2014
These are types that are in exported type signatures, but are not
exported themselves, e.g.

    struct Foo { ... }

    pub fn bar() -> Foo { ... }

will warn about the Foo.

Such types are not listed in documentation, and cannot be named outside
the crate in which they are declared, which is very user-unfriendly.

cc #10573.
@huonw
Copy link
Member

huonw commented Mar 1, 2014

#12595 added a lint for non-exported types in exported type signatures (e.g. struct Foo; pub struct Bar { pub x: Foo }) which will hopefully reduce the number of instances in the wild where deciding on a definite answer for this will change compiler behaviour.

@alexcrichton
Copy link
Member Author

I'm going to close this now that rust-lang/rfcs#136 has been accepted. Our "refinement" was to disallow this pattern entirely.

@glaebhoerl
Copy link
Contributor

@alexcrichton Due to the subsequent rust-lang/rfcs#200 you can still formulate all of the original "problematic examples" as just "public type in private module" rather than "private type".

E.g., where we had:

struct Foo { ... }; // private

impl Foo {
    pub fn id(&self) -> int;
}

pub fn some_foo() -> Foo { ... }

// can a user write `some_foo().id()`, even though they cannot see `Foo`?

We can't write that any more, but we can still write:

mod foo {  // private
    pub struct Foo { ... };

    impl Foo {
        pub fn id(&self) -> int;
    }
}

pub fn some_foo() -> foo::Foo { ... }

// can a user write `some_foo().id()`, even though they cannot see `foo::Foo` (because they cannot see `foo`)?

@japaric
Copy link
Member

japaric commented Nov 2, 2014

We can't write that any more, but we can still write:

Which can lead to linker errors as shown in this example (should look familiar!)

@alexcrichton
Copy link
Member Author

@glaebhoerl yes, I consider that part of the "refinement" in that it's just our answer to the question.

@japaric, indeed! I definitely consider that a compiler bug.

@glaebhoerl
Copy link
Contributor

@alexcrichton I don't understand - in particular what do you mean by "it" in "it's just our answer"? (My point was that all of the questions which used to exist (e.g. the ones listed in the OP) do still exist in some form.)

@alexcrichton
Copy link
Member Author

"it" being "public means you're tagged with pub", not "publicly accessible from other crates". Our answer was "to be an exposed member of something public, you must yourself be public".

We've decided that you can still reference visibly private types and functions and such, but they must be declared as public. Anything preventing you from doing this (such as the compiler linkage errors) are just that, compiler bugs.

@glaebhoerl
Copy link
Contributor

Yes, that much is clear. What I mean to ask is: are the answers to questions 1-5 here, and any other similar questions, substituting "public type in private module" for "private type", all going to be "yes"?

@alexcrichton
Copy link
Member Author

I think all 5 questions are answered by: If the struct is marked "pub", no, and if the struct is marked "pub", yes.

If the struct itself is a private type (nor marked pub), then the answer is all no.

flip1995 pushed a commit to flip1995/rust that referenced this issue Apr 6, 2023
…=Jarcho

Ignore `file!()` macro in `print_literal`, `write_literal`

changelog: [`print_literal`], [`write_literal`]: Ignore the `file!()` macro

`file!()` expands to a string literal with its span set to that of the `file!()` callsite, but isn't marked as coming from an expansion. To fix this we make sure we actually find a string/char literal instead of assuming it's one and slicing

It would also ignore any other macros that result in the same situation, but that shouldn't be common as `proc_macro::Span::call_site()` returns a span that is marked as from expansion

Fixes rust-lang#10544
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-visibility Area: Visibility / privacy. P-low Low priority
Projects
None yet
Development

No branches or pull requests

9 participants