Put a bound on collection misbehavior

[CAD97]

As currently written, when a logic error occurs in a collection's trait parameters, this allows completely arbitrary misbehavior, so long as it does not cause undefined behavior in std. However, because the extent of misbehavior is not specified, it is allowed for any code in std to start misbehaving in arbitrary ways which are not formally UB; consider the theoretical example of a global which gets set on an observed logic error. Because the misbehavior is only bound by not resulting in UB from safe APIs and the crate-level encapsulation boundary of all of std, this makes writing user unsafe code that utilizes std theoretically impossible, as it now relies on undocumented QOI (quality of implementation) that unrelated parts of std cannot be caused to misbehave by a misuse of std::collections APIs.

In practice, this is a nonconcern, because std has reasonable QOI and an implementation that takes advantage of this freedom is essentially a malicious implementation and only compliant by the most langauage-lawyer reading of the documentation.

To close this hole, we just add a small clause to the existing logic error paragraph that ensures that any misbehavior is limited to the collection which observed the logic error, making it more plausible to prove the soundness of user unsafe code.

This is not meant to be formal; a formal refinement would likely need to mention that values derived from the collection can also misbehave after a logic error is observed, as well as define what it means to "observe" a logic error in the first place. This fix errs on the side of informality in order to close the hole without complicating a normal reading which can assume a reasonable nonmalicious QOI.

See also discussion on IRLO.

r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs

This technically adds a new guarantee to the documentation, though I argue as written it's one already implicitly provided.

[rust-highfive]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with r? rust-lang/libs-api @rustbot label +T-libs-api -T-libs to request review from a libs-api team reviewer. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[CAD97]

I said..

@rustbot label -T-libs

[steffahn]

a formal refinement would likely need to mention that values derived from the collection can also misbehave after a logic error is observed

Another detail is that if the Eq/Hash/Ord implementations themselves access shared state, then encapsulation can be broken, too.

[CAD97]

I actually don't think that needs to be mentioned even in a formal refinement of misbehavior; obviously the code implementing the trait is being run, so whatever side effects it has happens.

[dtolnay]

@rust-lang/libs-api:
@rfcbot fcp merge

I am happy with the wording added here.

The IRLO thread linked from the PR description has a concrete example of code that must assume the guarantee being added here. The first post is sufficient to read; the rest of the thread is… long and not especially enlightening.

[rfcbot]

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

• @Amanieu
• @BurntSushi
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

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!

See this document for info about what commands tagged team members can give me.

[rfcbot]

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

[dtolnay]

@bors r+

[bors]

📌 Commit e6b1003 has been approved by dtolnay

[dtolnay]

@bors r+ rollup

[bors]

💡 This pull request was already approved, no need to approve it again.

• There's another pull request that is currently being tested, blocking this pull request: Clarify the guarantees of Vec::as_ptr and Vec::as_mut_ptr when there's no allocation #97521

[bors]

📌 Commit e6b1003 has been approved by dtolnay