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

Consider turning "visibility has no effect inside functions" into a lint #31776

Closed
sgrif opened this Issue Feb 19, 2016 · 7 comments

Comments

Projects
None yet
7 participants
@sgrif
Copy link
Contributor

sgrif commented Feb 19, 2016

The current code results in a hard error:

fn main() {
    pub struct foo;
}

This can be painful when writing macros that generate structs or inherent methods, which would otherwise need to be public (particularly when writing tests for those macros). This doesn't seem like something which should need to be an error, and could instead be a lint (which is deny by default). I'm unsure if this would need a full RFC or not, and was told that it'd be best to open an issue to start.

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Feb 19, 2016

Yeah, this restriction is pretty artificial, it isn't required for anything else.

@GuillaumeGomez

This comment has been minimized.

Copy link
Member

GuillaumeGomez commented Feb 22, 2016

I'm not sure a RFC is needed a small that little. cc @pnkfelix

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Feb 23, 2016

Yeah, this restriction is pretty artificial, it isn't required for anything else.

Moreover, the "visibility has no effect inside functions or block expressions" statement is not correct, pub is able to have effects inside of functions/blocks.

Example 1: Rustdoc.

Rustdoc wraps code snippets in fn main, so pub can't be used in examples in documentation.

Example 2: Inherent methods.

struct S;

mod m {
    fn f() {
        impl ::S {
            pub fn s(&self) {}
        }
    }
}

fn main() {
    S.s() // Privacy error, unless `fn s` is pub
}

Example 3: Pulling a function local type out of function through a public impl

pub trait Tr {
    type A;
}
pub struct S;

fn f() {
    struct Z;

    impl ::Tr for ::S {
        type A = Z; // Private-in-public error unless `struct Z` is pub
    }
}

fn main() {}

Example 4: Field of a function local type

trait Tr {
    type A;
    fn pull(&self) -> Self::A;
}
struct S;

mod m {
    fn f() {
        struct Z {
            field: u8
        }

        impl ::Tr for ::S {
            type A = Z;
            fn pull(&self) -> Self::A { Z{field: 10} }
        }
    }
}

type Tmp = <S as Tr>::A;

fn main() {
    let a = S.pull().field; // Privacy error unless `field: u8` is pub
}

Example 5: Any other private-in-public scenario in function local modules

fn f() {
    struct S;

    mod m {
        pub fn f() -> S { S } // Private-in-public error unless `struct S` is public
    }
}

fn main() {
}
@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Feb 23, 2016

I agree, that the examples above are contrived, but they are still possible.
My conclusion is that "pub in a function/block" may be a clippy lint, but it shouldn't be a hard language rule.

@pnkfelix pnkfelix added the T-lang label Feb 23, 2016

@pnkfelix

This comment has been minimized.

Copy link
Member

pnkfelix commented Feb 23, 2016

triage: I-nominated

(I suspect this is a P-medium issue.)

I suspect we can do this without an RFC (though I could imagine some of the details about interaction with private-in-public checking may lead to unanticipated results that could justify one...)

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Feb 25, 2016

This example makes me think that a lint would be wrong:

pub trait Tr {
    type A;
}
pub struct S;

fn f() {
    struct Z;

    impl ::Tr for ::S {
        type A = Z; // Private-in-public error unless `struct Z` is pub
    }
}

fn main() {}

After all, in this case you have to use pub (or move the impl) or the compiler contains. So maybe it should just be plain legal to put pub on a nested fn?

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Feb 25, 2016

Discussed in @rust-lang/lang meeting and conclusion was that we should just drop this restriction and not replace it with a lint, due to aforementioned examples.

triage: P-medium

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.