privacy: Fix type privacy holes when it doesn't cause too much breakage

[petrochenkov]

View all comments

A subset of #146470.

Private types in RPITITs now report hard errors.
Private types in bounds of associated types still only report the private_bounds lint due to unacceptable amount of breakage (#146470 (comment)).

Closes #144139

[petrochenkov]

@bors try

[petrochenkov]

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

[rustbot]

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 68 candidates
• Random selection from 16 candidates

[rust-bors]

☀️ Try build successful (CI)
Build commit: fcfb0ff (fcfb0ffc7ed7fc631df1136063939ef648434735, parent: 7ad4e69ad585d8ff214f7b42d01f1959eda08f40)

[petrochenkov]

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

[craterbot]

👌 Experiment pr-152543 created and queued.
🤖 Automatically detected try build fcfb0ff
🔍 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-152543 is now running

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

[craterbot]

🎉 Experiment pr-152543 is completed!
📊 27 regressed and 0 fixed (5514 total)
📊 267 spurious results on the retry-regressed-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-152543/retry-regressed-list.txt ↩

[petrochenkov]

Crater results are basically the same as #146470 without the second commit (see e.g. #146470 (comment)).

[petrochenkov]

Previous FCP - #146470 (comment).

[traviscross]

We talked about this in the lang call last week. @petrochenkov, it'd be helpful to us to have a brief write-up of what's being proposed here and what's changed since our FCP in #146470 (comment).

[petrochenkov]

and what's changed since our FCP in #146470 (comment).

Nothing is changed since the FCP in #146470 (comment).

[petrochenkov]

The previous summary (#146470 (comment)) is still relevant, except user-written private bounds on associated types are not turned into an error due to the amount breakage.

pub trait PubTrait {
  fn foo1() -> impl PrivTrait; // error now
  fn foo2() -> impl PubTrait<PrivType>; // error now

  type A1: PrivTrait; // not error, too much breakage (but reports `private_bounds` lint)
  type A2: PubTrait<PrivType>; // not error, too much breakage (but reports `private_bounds` lint)
}

[traviscross]

Thanks for that. For my part, I'm still motivated to fix the asymmetry with RPIT, so:

@rfcbot fcp merge lang

It is a bit inelegant that we're left, for now, with an asymmetry between RPITIT and associated types. If we were to turn out to need this space, e.g., to make a future feature sound, we'd need to fix that.

[rust-rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[scottmcm]

Making RPITIT match the RPIT behaviour makes sense to me.

(TBH it's not obvious to me that private bounds on an associated type are even a "problem" that we'd need to fix, but now that this isn't changing that, sure, happy to make it consistent in function return position. Especially if the public trait is sealed, having various non-public bounds on things doesn't seem unreasonable.)

@rfcbot reviewed

[tmandry]

Agreed with @scottmcm. I'm happy to make things more consistent and save space for another model in the future. Not sure we'll need this restriction in the long run.

@rfcbot reviewed

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[nikomatsakis]

Soooooo I still feel like I am missing a mental model for what we are attempting to achieve with our rules around privacy and private traits. It makes all of these changes feel kind of random to me. That said, I think we previously decided to move forward here, and this seems consistent with that, and RPITIT/RPIT consistency is good, and it seems like this will give us room to make more decisions in the future, so....

@rfcbot reviewed