Revert "Do not check privacy for RPITIT."

[mladedav]

The changes here were first merged in #143357 and later reverted in #144098 as it introduces new hard errors. There was a crater run tracked in #144139 to see how much projects would be broken (not that many, a few repositories on github are affected).

This reenables hard errors for privacy in RPITIT.

Fixes #143531
Closes #144139
Hopefully closes #71043

[rustbot]

compiler-errors is not on the review rotation at the moment.
They may take a while to respond.

[compiler-errors]

r? cjgillot

[mladedav]

Gentle ping @cjgillot

[joshtriplett]

This came up in today's @rust-lang/lang meeting. It's clear why this needed an FCP (as it's a breaking change), but we didn't feel like we had the context. Could we get a clear ask for what exactly the new hard error is that we're reviewing?

Does this just make it a hard error to write a public trait that has something like -> impl Trait using a private trait? Or is there more to it than that? The discussion in #144139 makes it sound like it's substantially more complex and subtle than that.

[mladedav]

It is a little bit more subtle in the current form, the main weirdness I remember is that creating a required method returning a private impl trait does not error out, only providing an implementation does, so

pub trait Foo {
    fn required_impl_trait() -> impl Private;
}

does not error while

pub trait Foo {
    fn required_impl_trait() -> impl Private;
}

impl Foo for S {
    fn required_impl_trait() -> impl Private { X }
}

and

pub trait Foo {
    fn provided_impl_trait() -> impl Private { X }
}

both error out.

The error is also reported when the Private trait is used in generic bounds of the method.

And then for AFIT it seems to work the same after desugaring, so async fn required_async_concrete() -> PrivateStruct; works the same way as it desugars to returning an impl trait which is considered private due to its private associated type. So declaring the method in the trait is not linted while implementing it is.

As I understand it, this is not as strict as it should be based on @petrochenkov's comment and even the first case of defining the trait should be rejected.

Here is a playground with more cases to see what does and does not produce errors (though the errors are just comments but compiling the code on this branch should provide the stated results).

---

To summarize, this adds errors when using a private trait in RPITIT but when the offending trait is not used in a trait bound and an implementation is not provided, there is a false negative an the error is not emitted even though it should be.

[traviscross]

@bors2 try

[traviscross]

@mladedav: I'm having trouble working out the reason why we'd give a hard error for the RPIT-in-trait-impl,

trait PrivTr {}
impl PrivTr for () {}
#[expect(private_bounds)]
pub trait PubTr {
    fn f1() -> impl PrivTr;
}
impl<T> PubTr for T {
    #[expect(private_interfaces)]
    fn f1() -> impl PrivTr {}
    //~^ error[E0446]: private trait `PrivTr` in public interface
    //~| help: can't leak private trait
}

given that we don't give an error for an RPIT-in-free-function,

trait PrivTr {}
impl PrivTr for () {}

#[expect(private_interfaces)]
pub fn f2() -> impl PrivTr {} //~ OK

and given that we allow the comparable associated type desugaring of the RPITIT:

trait PrivTr {}
impl PrivTr for () {}
pub trait PubTr {
    #[expect(private_bounds)]
    type F1: PrivTr; //~ OK
    fn f1() -> Self::F1;
}
impl<T> PubTr for T {
    type F1 = ();
    fn f1() -> Self::F1 {}
}

What's the rationale here?

cc @petrochenkov

---

I note that on nightly we give an error for this, when desugaring the RPIT-in-trait-impl to ATPIT:

#![feature(impl_trait_in_assoc_type)]
trait PrivTr {}
impl PrivTr for () {}
pub trait PubTr {
    #[expect(private_bounds)]
    type F1: PrivTr; //~ OK
    fn f1() -> Self::F1;
}
impl<T> PubTr for T {
    type F1 = impl PrivTr;
    //~^ error[E0446]: private trait `PrivTr` in public interface
    fn f1() -> Self::F1 {}
}

What's the rationale here? It makes sense why we can't leak a private type in this way -- we'd then be allowing a private type to be named. Why does this rise to the level of a hard error for a private trait in an impl trait bound?

---

Also, on the PR, I notice that placing the expect over the trait item doesn't work.

trait PrivTr {}
impl PrivTr for () {}
pub trait PubTr {
    #[expect(private_bounds)] //~ warning: this lint expectation is unfulfilled
    fn f1() -> impl PrivTr;
    //~^ warning: trait `PrivTr` is more private than the item `PubTr::f1::{anon_assoc#0}`
}

Should it?

[rust-bors]

☀️ Try build successful (CI)
Build commit: e117153 (e117153a45c546e883c1f91d82611775fcaeffe0, parent: c90bcb9571b7aab0d8beaa2ce8a998ffaf079d38)

[traviscross]

@craterbot check

[craterbot]

👌 Experiment pr-146470 created and queued.
🤖 Automatically detected try build e117153
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-146470 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-146470 is completed!
📊 27 regressed and 2 fixed (731634 total)
📊 2113 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-146470/retry-regressed-list.txt ↩

[traviscross]

@craterbot check p=1 crates=https://crater-reports.s3.amazonaws.com/pr-146470/retry-regressed-list.txt

[craterbot]

👌 Experiment pr-146470-1 created and queued.
🤖 Automatically detected try build e117153
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-146470-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-146470-1 is completed!
📊 20 regressed and 0 fixed (2070 total)
📊 122 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-146470-1/retry-regressed-list.txt ↩

[traviscross]

@craterbot check p=1 crates=https://crater-reports.s3.amazonaws.com/pr-146470-1/retry-regressed-list.txt

[craterbot]

👌 Experiment pr-146470-2 created and queued.
🤖 Automatically detected try build e117153
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-146470-2 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-146470-2 is completed!
📊 18 regressed and 0 fixed (142 total)
📊 39 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-146470-2/retry-regressed-list.txt ↩

[petrochenkov]

and given that we allow the comparable associated type desugaring of the RPITIT:

That's a bug in the implementation of rule #144139 (comment).
Fix - c5221eb.

[petrochenkov]

Also, on the PR, I notice that placing the expect over the trait item doesn't work.

The associated type item newly added for the RPITIT is used as a lint node, which is a different node from fn f1 to which the expect attribute is attached.

[petrochenkov]

It makes sense why we can't leak a private type in this way -- we'd then be allowing a private type to be named. Why does this rise to the level of a hard error for a private trait in an impl trait bound?

So, an associated type's interface can contain

• Generics
• Predicates
• Default type
• Bounds

type Type<T: PrivTrait /*predicates*/ = PrivTy /*generics*/>: PrivTrait /*bounds*/
    where Something: PrivTrait /*predicates*/
    = PrivTy /*default type*/;

And we just don't differentiate and check all of them when reporting the hard error version of the priv-in-pub check.
Originally that was simply a conservative choice allowing to avoid thinking too much about which of those can actually leak something private.
Maybe predicates and bounds cannot actually leak anything, then they can be demoted to a lint, we just need to have some more or less convincing proof of that written.

Why this is a hard error in the first place (rule #144139 (comment))?
During post-factum type privacy checks when we see <Type as Trait>::Assoc we cannot see and check its underlying type (that's the technical limitation mentioned in the RFC), we can only check that Type and Trait are not private.
So at definition site we must ensure (not just lint) that impl Trait for Type cannot define an Assoc that is more private than any of them.

[traviscross]

Thanks, that makes sense and is helpful. If you could, help me distinguish these cases according to that, or let me know if you think any of these behaviors are incorrect under this model. In particular, I'm suspicious of the hard error in the first case given the other two.

A private trait in the bound of an impl Trait type that's used as the definition of an associated type in an impl raises an error (the associated type, in the trait, uses a public supertrait as the bound):

#![feature(impl_trait_in_assoc_type)]
pub trait Super {}
trait PrivTr: Super {}
impl Super for () {}
impl PrivTr for () {}
pub trait PubTr {
    type F1: Super;
    fn f1() -> Self::F1;
}
impl<T> PubTr for T {
    type F1 = impl PrivTr;
    //~^ error[E0446]: private trait `PrivTr` in public interface
    fn f1() -> Self::F1 {}
}

Similar to the above but with the impl Trait, with the bound that includes the private trait, pulled out to a public type alias.

#![feature(impl_trait_in_assoc_type, type_alias_impl_trait)]
pub trait Super {}
trait PrivTr: Super {}
impl Super for () {}
impl PrivTr for () {}
pub trait PubTr {
    type F1: Super;
    fn f1() -> Self::F1;
}
#[expect(private_interfaces)]
pub type Alias = impl PrivTr;
#[define_opaque(Alias)]
fn defines() -> Alias {}
impl<T> PubTr for T {
    type F1 = Alias; //~ OK
    fn f1() -> Self::F1 { defines() }
}

RPIT-in-free-function:

trait PrivTr {}
impl PrivTr for () {}
#[expect(private_interfaces)]
pub fn f2() -> impl PrivTr {} //~ OK

[petrochenkov]

Here

type F1 = Alias; //~ OK

the compiler cannot normalize properly, and then see and check the underlying type of Alias.
Otherwise it would produce an error.

So either the normalization should be implemented properly (#126076), or type_alias_impl_traits (like pub type Alias = impl PrivTr;) should follow the same rule as associated types and produce hard errors instead of lints at definition site.
Thankfully, such free type aliases with normalization issues are not yet stable.

The "RPIT-in-free-function" is ok, it's not an associated type context, so it's a lint and not an error.

[traviscross]

Makes sense. Thanks for helping me see the model more clearly.