Remove unnecessary trait accessibility check.

[ghost]

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?

[rust-highfive]

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

[bluss]

Not sure what the behavior should be if the trait has a private supertrait.

[ghost]

I don't think this affects anything in that regard. It's already possible to reexport traits that depend on private supertraits anyway, e.g.:

pub use inner::Bar;

mod inner {
    trait Foo {
        fn foo(&self) { println!("foo"); }
    }
    pub trait Bar: Foo {
        fn bar(&self) { println!("bar"); }
    }
    impl Foo for i32 {}
    impl Bar for i32 {}
}

fn main() {
    //0i32.foo(); // Can't call this
    0i32.bar();
}

Playpen

[bluss]

Ok, I thought you could call .foo() if you remove the accessibility check.

By the way, you can call foo through Bar::foo.

[ghost]

Don't you think you can call .foo() if you remove the accessibility check?

Nope, still gives the same error as on playpen:

test.rs:15:10: 15:15 error: no method named `foo` found for type `i32` in the current scope
test.rs:15     0i32.foo();
                    ^~~~~
test.rs:15:10: 15:15 help: items from traits can only be used if the trait is in scope; the following trait is implemented but not in scope, perhaps add a `use` for it:
test.rs:15:10: 15:15 help: candidate #1: use `inner::Foo`
error: aborting due to previous error

By the way, you can call foo through Bar::foo.

This also still works.

[bluss]

Ok, I'll report separate bugs for the Bar::foo problem.

[alexcrichton]

It's been awhile since I've poked around the privacy-related code, but this looks good to me. To me it makes sense that the method call will only be resolved if the trait is already in scope, and because the import of the trait already had the privacy check run on it and verified then we don't have private trait methods so this is fine to let through.

That being said I'd like a second set of eyes on this, so:

r? @nrc

[nrc]

Does this still give an error?

mod foo {
    trait Bar {
        fn baz() {}
    }

    impl Bar for i32 {}
}

fn main() {
    <i32 as ::foo::Bar>::baz(); //~ERROR method `baz` is inaccessible
}

I assume that at the least the error message changes (to something about no impl for i32 in scope). Could you add a c-fail test (with some explanation) for this please?

[ghost]

Added a cfail test. The error is exactly the same as on latest stable/beta/nightly:

privacy-ufcs.rs:22:13: 22:29 error: method `baz` is inaccessible
privacy-ufcs.rs:22     <i32 as ::foo::Bar>::baz(); //~ERROR method `baz` is inaccessible
                               ^~~~~~~~~~~~~~~~
privacy-ufcs.rs:22:13: 22:29 note: trait `Bar` is private
privacy-ufcs.rs:22     <i32 as ::foo::Bar>::baz(); //~ERROR method `baz` is inaccessible
                               ^~~~~~~~~~~~~~~~
error: aborting due to previous error

[nrc]

@bors: r+

[bors]

📌 Commit b3e1aca has been approved by nrc

[bors]

⌛ Testing commit b3e1aca with merge ad82e0a...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

[petrochenkov]

Hmm, there's a regression in current nightly (it may be intentional though):

mod m {
    trait Priv {
        fn f(&self) {}
    }
    impl Priv for super::S {}
    pub trait Pub: Priv {}
}

struct S;
impl m::Pub for S {}

fn g<T: m::Pub>(arg: T) {
    arg.f();
}

fn main() {
    // Compiles on nightly, reports error "source trait is private" on stable and beta
    g(S);
}

I guess it was caused by this patch?
cc @alexcrichton

[bluss]

@petrochenkov That behavior is nownot correct according to the discussion in #28514

[bluss]

Since all regressions should be tracked, do we need an issue for that? cc @brson

[alexcrichton]

Thanks for the sharp eyes @petrochenkov! I've opened a PR to revert: #29325