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

Trait re-exports fail due to privacy of containing module #18241

Closed
Ryman opened this Issue Oct 22, 2014 · 16 comments

Comments

Projects
None yet
@Ryman
Contributor

Ryman commented Oct 22, 2014

This was working previously before rustc 0.13.0-nightly (3d2cf6063 2014-10-22 00:22:04 +0000).

If this is an intended change, we should probably give the error where the re-export is defined and not on use of the trait.

use foo::Bar;

mod foo {
    pub use self::bar::Bar;

    mod bar {
        pub trait Bar {
            fn call(&self) -> bool;
        }
    }
}

impl foo::Bar for uint {
    fn call(&self) -> bool {
        true
    }
}

fn main() {
    println!("{}", 0u.call())
    //^~ ERROR source trait is inaccessible
}
@jansegre

This comment has been minimized.

Show comment
Hide comment
@jansegre

jansegre Oct 27, 2014

I've noticed this too while compiling nphysics, I've narrowed it to http://is.gd/X0EHDP

The error is

<anon>:22:5: 22:13 error: source trait is inaccessible
<anon>:22     b.boom();
              ^~~~~~~~
<anon>:22:5: 22:13 note: module `bar` is private
<anon>:22     b.boom();
              ^~~~~~~~
error: aborting due to previous error

I'm using rustc 0.13.0-nightly (f03745244 2014-10-26 16:42:33 +0000).

I've noticed this too while compiling nphysics, I've narrowed it to http://is.gd/X0EHDP

The error is

<anon>:22:5: 22:13 error: source trait is inaccessible
<anon>:22     b.boom();
              ^~~~~~~~
<anon>:22:5: 22:13 note: module `bar` is private
<anon>:22     b.boom();
              ^~~~~~~~
error: aborting due to previous error

I'm using rustc 0.13.0-nightly (f03745244 2014-10-26 16:42:33 +0000).

@zargony

This comment has been minimized.

Show comment
Hide comment
@zargony

zargony Jan 23, 2015

Contributor

I'm facing the same problem for quite some time now (currently rustc 1.0.0-nightly (29bd9a0 2015-01-20 23:03:09 +0000)). Is this a bug or is it intended behavior?

Contributor

zargony commented Jan 23, 2015

I'm facing the same problem for quite some time now (currently rustc 1.0.0-nightly (29bd9a0 2015-01-20 23:03:09 +0000)). Is this a bug or is it intended behavior?

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Jan 29, 2015

Member

@nikomatsakis is this expected?

Member

steveklabnik commented Jan 29, 2015

@nikomatsakis is this expected?

bors added a commit that referenced this issue Sep 22, 2015

Auto merge of #28504 - Eljay:fix-trait-privacy, r=nrc
Fixes #16264 / #18241.

As far as I can tell, it should be impossible for a trait to be inaccessible if it's in scope, so this check is unnecessary. Are there any cases where this check is actually needed?

@huonw huonw added the A-resolve label Oct 20, 2015

@Syntaf

This comment has been minimized.

Show comment
Hide comment
@Syntaf

Syntaf Oct 20, 2015

Just ran into this bug in my code, would love to see a fix for this!

Syntaf commented Oct 20, 2015

Just ran into this bug in my code, would love to see a fix for this!

@zargony

This comment has been minimized.

Show comment
Hide comment
@zargony

zargony Oct 21, 2015

Contributor

Im also still running into this from time to time (rustc 1.3.0). I'd love to see it fixed or clarified whether this is intended or a bug.

Contributor

zargony commented Oct 21, 2015

Im also still running into this from time to time (rustc 1.3.0). I'd love to see it fixed or clarified whether this is intended or a bug.

@AndiDog

This comment has been minimized.

Show comment
Hide comment
@AndiDog

AndiDog Oct 28, 2015

Contributor

I also have this with rustc nightly (from PPA: "201510280406e0e2627wily")

Contributor

AndiDog commented Oct 28, 2015

I also have this with rustc nightly (from PPA: "201510280406e0e2627wily")

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Oct 28, 2015

Contributor

@steveklabnik I think this is a bug, actually. @nrc @alexcrichton do you agree?

Contributor

nikomatsakis commented Oct 28, 2015

@steveklabnik I think this is a bug, actually. @nrc @alexcrichton do you agree?

@nikomatsakis

This comment has been minimized.

Show comment
Hide comment
@nikomatsakis

nikomatsakis Oct 28, 2015

Contributor

@steveklabnik yes I realize I'm responding to a comment from Jan 28 :) a bit late, I know

Contributor

nikomatsakis commented Oct 28, 2015

@steveklabnik yes I realize I'm responding to a comment from Jan 28 :) a bit late, I know

@Eljay

This comment has been minimized.

Show comment
Hide comment
@Eljay

Eljay Oct 28, 2015

Contributor

@nikomatsakis this was "fixed" by #28504 but reverted by #29325 due to a regression.

Personally, I think that the correct fix for the regression should be to make pub trait Pub: Priv a privacy error, but as far as I can tell, it's not easy to implement this currently, and it's likely to cause further regressions. Trait privacy is hard :(

Contributor

Eljay commented Oct 28, 2015

@nikomatsakis this was "fixed" by #28504 but reverted by #29325 due to a regression.

Personally, I think that the correct fix for the regression should be to make pub trait Pub: Priv a privacy error, but as far as I can tell, it's not easy to implement this currently, and it's likely to cause further regressions. Trait privacy is hard :(

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Oct 28, 2015

Member

@nikomatsakis better late than never ❤️

Member

steveklabnik commented Oct 28, 2015

@nikomatsakis better late than never ❤️

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Oct 28, 2015

Member

@nikomatsakis yeah this looks like a bug to me, and I think @Eljay is right in that #28504 is probably the right fix if we deal with trait Pub: Priv, which I think is the cause of some other visibility/privacy bug around here...

Member

alexcrichton commented Oct 28, 2015

@nikomatsakis yeah this looks like a bug to me, and I think @Eljay is right in that #28504 is probably the right fix if we deal with trait Pub: Priv, which I think is the cause of some other visibility/privacy bug around here...

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Dec 23, 2015

Contributor

This can be simplified even further to: http://is.gd/QG9LGU. Also an additional note, this only affects usage in the same crate. Other crates are able to use the re-exports fine.

Contributor

sgrif commented Dec 23, 2015

This can be simplified even further to: http://is.gd/QG9LGU. Also an additional note, this only affects usage in the same crate. Other crates are able to use the re-exports fine.

@LukasKalbertodt

This comment has been minimized.

Show comment
Hide comment
@LukasKalbertodt

LukasKalbertodt Jan 25, 2016

Contributor

Still a bug in 1.6 😢

Contributor

LukasKalbertodt commented Jan 25, 2016

Still a bug in 1.6 😢

@sorpaas

This comment has been minimized.

Show comment
Hide comment

sorpaas commented Feb 7, 2016

+1

@gbersac

This comment has been minimized.

Show comment
Hide comment
@gbersac

gbersac Feb 16, 2016

still a bug in cargo 0.8.0-nightly (28a0cbb 2016-01-17)

gbersac commented Feb 16, 2016

still a bug in cargo 0.8.0-nightly (28a0cbb 2016-01-17)

@tomaka

This comment has been minimized.

Show comment
Hide comment
@tomaka

tomaka Mar 1, 2016

Contributor

For those who may fall on this issue, it can be bypassed by using the UFCS syntax: TraitName::method(&trait_implementor, params...).

For example http://is.gd/QG9LGU becomes http://is.gd/zYo5Qm

Contributor

tomaka commented Mar 1, 2016

For those who may fall on this issue, it can be bypassed by using the UFCS syntax: TraitName::method(&trait_implementor, params...).

For example http://is.gd/QG9LGU becomes http://is.gd/zYo5Qm

bors added a commit that referenced this issue Mar 4, 2016

Auto merge of #31920 - jseyfried:fix_spurious_privacy_error, r=nikoma…
…tsakis

This PR allows using methods from traits that are visible but are defined in an inaccessible module (fixes #18241). For example,
```rust
mod foo {
    pub use foo::bar::Tr;
    mod bar { // This module is inaccessible from `g`
        pub trait Tr { fn f(&self) {} }
    }
}
fn g<T: foo::Tr>(t: T) {
    t.f(); // Currently, this is a privacy error even though `foo::Tr` is visible
}
```

After this PR, it will continue to be a privacy error to use a method from a trait that is not visible. This can happen when a public trait inherits from a private trait (in violation of the `public_in_private` lint) -- see @petrochenkov's example in #28504.
r? @nikomatsakis

bors added a commit that referenced this issue Mar 6, 2016

Auto merge of #31920 - jseyfried:fix_spurious_privacy_error, r=nikoma…
…tsakis

This PR allows using methods from traits that are visible but are defined in an inaccessible module (fixes #18241). For example,
```rust
mod foo {
    pub use foo::bar::Tr;
    mod bar { // This module is inaccessible from `g`
        pub trait Tr { fn f(&self) {} }
    }
}
fn g<T: foo::Tr>(t: T) {
    t.f(); // Currently, this is a privacy error even though `foo::Tr` is visible
}
```

After this PR, it will continue to be a privacy error to use a method from a trait that is not visible. This can happen when a public trait inherits from a private trait (in violation of the `public_in_private` lint) -- see @petrochenkov's example in #28504.
r? @nikomatsakis

@bors bors closed this in #31920 Mar 6, 2016

mattico added a commit to mattico/rustup.rs that referenced this issue May 12, 2017

Remove unnecessary pub mod
rust-lang/rust#18241 is fixed now, so this mod doesn't need to be pub.

bors added a commit to rust-lang-nursery/rustup.rs that referenced this issue May 18, 2017

Auto merge of #1116 - mattico:remove-pub-fixme, r=Diggsey
Remove unnecessary pub mod

rust-lang/rust#18241 is fixed now, so this mod doesn't need to be pub.

Possible downside: the minimum compiler version to build is now 1.9.0 or thereabouts.

bors added a commit to rust-lang-nursery/rustup.rs that referenced this issue May 18, 2017

Auto merge of #1116 - mattico:remove-pub-fixme, r=Diggsey
Remove unnecessary pub mod

rust-lang/rust#18241 is fixed now, so this mod doesn't need to be pub.

Possible downside: the minimum compiler version to build is now 1.9.0 or thereabouts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment