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

Possibly bogus private type warning with trait impls #32318

Closed
arcnmx opened this issue Mar 17, 2016 · 5 comments
Closed

Possibly bogus private type warning with trait impls #32318

arcnmx opened this issue Mar 17, 2016 · 5 comments

Comments

@arcnmx
Copy link
Contributor

arcnmx commented Mar 17, 2016

The following example triggers a E0446 complaining about a "private type in public interface" even though it's only used in a trait impl, and not actually exposed to the public.

pub trait T {
    fn f() -> u8;
}

type Alias = u8;
impl T for () {
    fn f() -> Alias { 0 }
}
<anon>:9:15: 9:20 warning: private type in public interface, #[warn(private_in_public)] on by default
<anon>:9     fn f() -> Alias { 0 }
                       ^~~~~
<anon>:9:15: 9:20 warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
<anon>:9:15: 9:20 note: for more information, see the explanation for E0446 (`--explain E0446`)

For some context, a more realistic usage pattern where I've seen this come up a few times is with associated types. This was valid before the change, though whether it was intentional or not I'm not sure... It seems reasonable to allow it to me:

pub trait T {
    type Error;

    fn f() -> Result<u8, Self::Error>;
}

pub struct MyError;
type MyResult<T> = Result<T, MyError>;
impl T for () {
    type Error = MyError;

    fn f() -> MyResult<u8> { Ok(0) }
}
@petrochenkov
Copy link
Contributor

Duplicate of #30503
(The fix is trivial, it only needs a decision from the lang team.)

@arcnmx
Copy link
Contributor Author

arcnmx commented Mar 17, 2016

While I'd consider them to be slightly different issues (private type aliases in public signatures vs type aliases used internally/hidden in trait impls) where one could be allowed while the other not... If #30503 is decided on positively it certainly would fix this!

My inclination is that as far as trait impls are concerned, this sort of checking should at most only apply to associated types and impl<> trait bounds. If we just go through with allowing them wholesale that works too, though!

@petrochenkov
Copy link
Contributor

type aliases used internally/hidden in trait impl

impl T for () is not internal/hidden, it's an impl of a public trait T for a public type (), therefore signatures of its items are checked for the presence of private items.

@arcnmx
Copy link
Contributor Author

arcnmx commented Mar 17, 2016

impl T for () is not internal/hidden, it's an impl of a public trait T for a public type (), therefore signatures of its items are checked for the presence of private items.

The impl itself is not hidden, but the inner implementation of it is. The public/exposed part is that the impl exists, any of its associated type bindings, and that it may have generic trait bounds. I guess the conceptual difference for me is that the type signature of an impl'd method can never really change; it's just not possible, even if you use a type alias in place of another type.

Guess I'm splitting hairs though, either way I certainly would like to see the ability to use type as a true transparent alias in private situations. Are type macros stable yet? Maybe that's a better solution to distinguish between type and an unnamed alias...

@Mark-Simulacrum
Copy link
Member

The original code now compiles without any warnings, so I'm going to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants