Skip to content
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

Bikeshed the name of our NoCell trait #994

Closed
jswrenn opened this issue Mar 1, 2024 · 4 comments · Fixed by #1137
Closed

Bikeshed the name of our NoCell trait #994

jswrenn opened this issue Mar 1, 2024 · 4 comments · Fixed by #1137

Comments

@jswrenn
Copy link
Collaborator

jswrenn commented Mar 1, 2024

Related to #251.

Possibilities:

  • Freeze
  • NoCell
  • InteriorImmutable
  • SharedImmutable
  • NoInteriorMutability
@jswrenn
Copy link
Collaborator Author

jswrenn commented Mar 1, 2024

I'm partial to SharedImmutable. It accurately describes that the type is immutable under shared aliasing.

This is advantage over InteriorImmutable and NoInteriorMutability, where the "under shared aliasing" is implicit.

@joshlf joshlf changed the title Bikeshed the name of our NoCell trait. Bikeshed the name of our NoCell trait Mar 1, 2024
@kupiakos
Copy link
Contributor

I personally find the name Freeze to be incredibly confusing and ambiguous, so I'd really prefer zerocopy (or Rust) not go with that.

Because UnsafeCells are documented as the core requirement for interior mutability, I'm a big fan of NoCell or NoUnsafeCell for being both explicit about the requirement and short. Brevity is more valuable than folks often think, especially when communicating with colleagues. "no cell" is two syllables, the other alternatives are 5+.

immutable under shared aliasing

Not just shared aliasing (being when two &T overlap), but shared borrows generally.

I'm partial to SharedImmutable.

One issue I have with this (and most of these names) is that the constraint is only for the struct itself containing an UnsafeCell, and not for pointers to interior mutable data, e.g. Box<UnsafeCell<T>>. So, an external user might see a trait in zerocopy called SharedImmutable and infer that to mean "the state of this object can never change through a &T", outside the trait's intended purpose. This isn't true because this trait could be implemented for all &T, including &Cell<T>.

Some proposals I've seen put the word "shallow" in to disambiguate, but again that makes the trait name even longer.

@joshlf
Copy link
Member

joshlf commented Mar 19, 2024

immutable under shared aliasing

Not just shared aliasing (being when two &T overlap), but shared borrows generally.

Can you say more about this distinction? I'm not sure what you're referring to.

I'm partial to SharedImmutable.

One issue I have with this (and most of these names) is that the constraint is only for the struct itself containing an UnsafeCell, and not for pointers to interior mutable data, e.g. Box<UnsafeCell<T>>. So, an external user might see a trait in zerocopy called SharedImmutable and infer that to mean "the state of this object can never change through a &T", outside the trait's intended purpose. This isn't true because this trait could be implemented for all &T, including &Cell<T>.

Some proposals I've seen put the word "shallow" in to disambiguate, but again that makes the trait name even longer.

I agree that this is a problem, although I'll note that NoCell suffers from this as well.

@kupiakos
Copy link
Contributor

I agree that this is a problem, although I'll note that NoCell suffers from this as well.

Agreed, though I'd (weakly) argue it's to a lesser degree. It refers to the presence of a particular struct rather than whether state can be shared-mutated, and I'd suggest that makes it less likely to conflate Cell<T> and &Cell<T>.

I've been thinking about the "shallow" qualifier too, and it has its own problem: shallow could mean "no pointer indirection" or it could mean "is not wrapped in another type". Same with "direct" and "plain".

Some other qualifier ideas:

  • NoLocalCell
    • I think this is my favorite? "local" implies non-pointer and doesn't suggest "no wrapper types" in quite the way "shallow" might.
  • NoNonPtrCell
  • NoCellInLayout
  • A coworker suggests NoContainedCell.

Can you say more about this distinction? I'm not sure what you're referring to.

It's (pedantic) pushback on using the term "aliasing" for all &T. Aliasing &T refer to overlapping segments of memory. Two &T bindings are allowed to alias, but can either be aliasing or non-aliasing. While technically you could all say any &T binding is "aliasing" the owning binding by being another way to refer to the same memory, the term loses its utility when used this way - &mut T is then also aliasing.
So, "immutable under shared aliasing" is overly specific: "immutable under shared borrows" or "immutable under shared references" is more correct.

jswrenn added a commit that referenced this issue Apr 24, 2024
We selected `Immutable` on @rcoh's suggestion that our name for this be a
single, evocative word, like `Send` and `Sync`. As with `Send` and `Sync`,
we do not hope that `Immutable`, on its own, captures the entire semantics
of this trait; the burden of entirely capturing its semantics rests on its
documentation.

We are not overly concerned with the risk of future conflict with Rust. If
and when Rust stabilizes its own version of this trait, we will adjust
zerocopy to use Rust's trait instead.

Closes #994
jswrenn added a commit that referenced this issue Apr 24, 2024
We selected `Immutable` on @rcoh's suggestion that our name for this be a
single, evocative word, like `Send` and `Sync`. As with `Send` and `Sync`,
we do not hope that `Immutable`, on its own, captures the entire semantics
of this trait; the burden of entirely capturing its semantics rests on its
documentation.

We are not overly concerned with the risk of future conflict with Rust. If
and when Rust stabilizes its own version of this trait, we will adjust
zerocopy to use Rust's trait instead.

Closes #994
jswrenn added a commit that referenced this issue Apr 24, 2024
We selected `Immutable` on @rcoh's suggestion that our name for this be a
single, evocative word, like `Send` and `Sync`. As with `Send` and `Sync`,
we do not hope that `Immutable`, on its own, captures the entire semantics
of this trait; the burden of entirely capturing its semantics rests on its
documentation.

We are not overly concerned with the risk of future conflict with Rust. If
and when Rust stabilizes its own version of this trait, we will adjust
zerocopy to use Rust's trait instead.

Closes #994
github-merge-queue bot pushed a commit that referenced this issue Apr 24, 2024
We selected `Immutable` on @rcoh's suggestion that our name for this be a
single, evocative word, like `Send` and `Sync`. As with `Send` and `Sync`,
we do not hope that `Immutable`, on its own, captures the entire semantics
of this trait; the burden of entirely capturing its semantics rests on its
documentation.

We are not overly concerned with the risk of future conflict with Rust. If
and when Rust stabilizes its own version of this trait, we will adjust
zerocopy to use Rust's trait instead.

Closes #994
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 a pull request may close this issue.

3 participants