Skip to content

[pointer] Clarify semantics of aliasing invariants (#1889) #2378

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

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented Feb 24, 2025

Previously, we supported the AtLeast bound, which was used to describe a subset relationship in which I: AtLeast<J> implied that I as at least as restrictive as J. However, as described in #1866, this incorrectly models invariants as monotonic. In reality, invariants both provide guarantees but also require guarantees.

This commit takes a step in the direction of resolving #1866 by removing AtLeast. Uses of AtLeast<Shared> are replaced by a new Reference trait, which is implemented for Shared and Exclusive. This serves two purposes: First, it makes it explicit what this bound means. Previously, AtLeast<Shared> had an ambiguous meaning, while Reference means precisely that an invariant is either Shared or Exclusive and nothing else. Second, it paves the way for #1183, in which we may add new aliasing invariants which convey ownership. In that case, it will be important for existing methods to add Reference bounds when those methods would not be sound in the face of ownership semantics.

We also inline the items in the invariant module, which were previously generated by macro. The addition of the Reference trait did not play nicely with that macro, and we will likely need to go further from the macro in order to fix #1839 – this fix will likely require making aliasing invariants meaningfully different than other invariants, for example by adding an associated type.

Makes progress on #1866

gherrit-pr-id: I86ef959643b97a34da81bf55a1fed5060b9cf6b2

@joshlf joshlf requested a review from jswrenn February 24, 2025 21:01
@joshlf
Copy link
Member Author

joshlf commented Feb 24, 2025

Backporting #1889

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.82%. Comparing base (393d8ea) to head (6617f30).

Additional details and impacted files
@@            Coverage Diff             @@
##           v0.8.x    #2378      +/-   ##
==========================================
+ Coverage   87.47%   87.82%   +0.34%     
==========================================
  Files          16       16              
  Lines        6182     6159      -23     
==========================================
+ Hits         5408     5409       +1     
+ Misses        774      750      -24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Previously, we supported the `AtLeast` bound, which was used to describe
a subset relationship in which `I: AtLeast<J>` implied that `I` as at
least as restrictive as `J`. However, as described in #1866, this
incorrectly models invariants as monotonic. In reality, invariants both
provide guarantees but also *require* guarantees.

This commit takes a step in the direction of resolving #1866 by removing
`AtLeast`. Uses of `AtLeast<Shared>` are replaced by a new `Reference`
trait, which is implemented for `Shared` and `Exclusive`. This serves
two purposes: First, it makes it explicit what this bound means.
Previously, `AtLeast<Shared>` had an ambiguous meaning, while
`Reference` means precisely that an invariant is either `Shared` or
`Exclusive` and nothing else. Second, it paves the way for #1183, in
which we may add new aliasing invariants which convey ownership. In that
case, it will be important for existing methods to add `Reference`
bounds when those methods would not be sound in the face of ownership
semantics.

We also inline the items in the `invariant` module, which were
previously generated by macro. The addition of the `Reference` trait did
not play nicely with that macro, and we will likely need to go further
from the macro in order to fix #1839 – this fix will likely require
making aliasing invariants meaningfully different than other invariants,
for example by adding an associated type.

Makes progress on #1866

gherrit-pr-id: I86ef959643b97a34da81bf55a1fed5060b9cf6b2
@joshlf joshlf added this pull request to the merge queue Feb 24, 2025
Merged via the queue into v0.8.x with commit 32fdbee Feb 24, 2025
87 checks passed
@joshlf joshlf deleted the v0.8.x-1889 branch February 24, 2025 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants