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

Privacy doesn't consider type/value namespaces. #4110

Closed
brson opened this issue Dec 4, 2012 · 12 comments · Fixed by #12245
Closed

Privacy doesn't consider type/value namespaces. #4110

brson opened this issue Dec 4, 2012 · 12 comments · Fixed by #12245
Labels
A-traits Area: Trait system A-typesystem Area: The type system P-medium Medium priority
Milestone

Comments

@brson
Copy link
Contributor

brson commented Dec 4, 2012

The privacy passes don't consider the type/value namespaces that resolve has, so you can use a private thing in a module so long as there's a public thing with the same name in a different namespace.

Original bug report

Static method on non-pub impl makes pub type private

use foo::Bar does not resolve unless the static method is removed or the impl Bar is marked pub.

pub mod foo {
    pub enum Bar {
        Baz
    }

    impl Bar {
        static fn where() { }
    }
}

fn main() {
    use foo::Bar;
}
@brson
Copy link
Contributor Author

brson commented Dec 4, 2012

It doesn't require a static method at all. A priv function with the same name will cause the same issue:

pub mod foo {
    pub enum Bar {
        Baz
    }

    fn Bar() { }
}

fn main() {
    use foo::Bar;
}

@pcwalton
Copy link
Contributor

Why is this backwards incompatible?

@pcwalton
Copy link
Contributor

Nominating for some milestone that isn't backwards compatible.

@emberian
Copy link
Member

Same testcase reproduces with:

foo.rs:10:8: 10:16 error: unresolved import: found `Bar` in `foo` but it is private
foo.rs:10     use foo::Bar;
                  ^~~~~~~~
foo.rs:10:8: 10:16 error: failed to resolve import `foo::Bar`
foo.rs:10     use foo::Bar;
                  ^~~~~~~~
error: aborting due to 2 previous errors

@graydon
Copy link
Contributor

graydon commented Jul 18, 2013

accepted for production-ready milestone

@huonw
Copy link
Member

huonw commented Sep 18, 2013

Triage: @cmr's testcase still gives that error.

@pnkfelix
Copy link
Member

Accepting for 1.0

@alexcrichton
Copy link
Member

This program has an error in the proper location

pub mod foo {         
    pub enum Bar {    
        Baz           
    }                 

    impl Bar {        
        fn where() { }
    }                 
}                     

fn main() {           
    use foo::Bar;     
    Bar::where(); // error here
}                     

Whereas this program does not generate an error where it should:

pub mod foo {
    pub enum Bar {
        Baz
    }

    fn Bar() { }
}

fn main() {
    use foo::Bar;

    Bar();
}

Updating the title/description to what I believe the bug is.

@nrc
Copy link
Member

nrc commented Feb 4, 2014

Yeah, the bug seems to be because use foo::Bar makes the enum Bar visible, but also fn Bar visible. It seems to me that the issue is that use foo::Bar is ambiguous here - in fact the resolution of the Bar searches first types, then functions, so the following program which ought to be OK, is not,

pub mod foo {
    enum Bar {
        Baz
    }

    pub fn Bar() { }
}

fn main() {
    use foo::Bar;

    Bar();
}

So, the question is should we allow use statements which are ambiguous due to the type/fn namespacing? If so, what is the expected behaviour here? - Do we get types first but need to give an error on Bar() because it should not be visible (as opposed to not public)? If we want to disallow ambiguous use statements should we error always or only if the items we are 'use'ing have different privacy annotations?

@alexcrichton
Copy link
Member

This is a tough problem that the privacy infrastructure isn't quite designed to handle today. I erroneously wrote it with the idea that once a name was in your scope it was fine to use.

I think that we must allow the import use foo::Bar; is valid, and we'll have to warn at the use-site. I believe that this is possible by maintaining whether the type or value is public/private for a particular name in scope, and then whichever one you use you use that chain of privacy. The details are a little fuzzy, but that's what I think that the ideal world would be.

In the meantime (for the sake of backwards-compatibility), we could disallow the use foo::Bar import. This isn't the right behavior, but it won't break any code once we fix the bug (in the future).

@nrc
Copy link
Member

nrc commented Feb 4, 2014

So, the semantics for |use path| should be that it imports all items with name indicated by path, but we maintain separate privacy information for each item.

@alexcrichton
Copy link
Member

In the ideal world, I think that's how we'll have to do it. The import could be valid, but it could also be invalid, it depends on how you use it afterwards.

The resolve pass already handles this all correctly, it's just transferring the information to privacy that's the hard part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system A-typesystem Area: The type system P-medium Medium priority
Projects
None yet
8 participants