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

Tracking Issue for Cell::as_array_of_cells #88248

Open
1 of 3 tasks
oconnor663 opened this issue Aug 23, 2021 · 12 comments
Open
1 of 3 tasks

Tracking Issue for Cell::as_array_of_cells #88248

oconnor663 opened this issue Aug 23, 2021 · 12 comments
Labels
A-array Area: `[T; N]` C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@oconnor663
Copy link
Contributor

oconnor663 commented Aug 23, 2021

Feature gate: #![feature(as_array_of_cells)]

This is a tracking issue for Cell::as_array_of_cells, the const-generic counterpart to the existing Cell::as_slice_of_cells.

Public API

impl<T, const N: usize> Cell<[T; N]> {
    pub const fn as_array_of_cells(&self) -> &[Cell<T>; N] { ... }
}

// Example

let mut array: [i32; 3] = [1, 2, 3];
let cell_array: &Cell<[i32; 3]> = Cell::from_mut(&mut array);
let array_cell: &[Cell<i32>; 3] = cell_array.as_array_of_cells();

Steps / History

Unresolved Questions

  • None yet.
@oconnor663 oconnor663 added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Aug 23, 2021
@workingjubilee workingjubilee added the A-array Area: `[T; N]` label Mar 7, 2023
@pierzchalski
Copy link
Contributor

@oconnor663 Do you know who would be the right person to @ in order to get this reviewed/FCP'd? Given the existence of as_slice_of_cells I don't think we'll have a blocking discussion on the name or semantics, at least.

@pierzchalski
Copy link
Contributor

Wait, can I just do @rfcbot needs-fcp

@TOETOE55
Copy link

TOETOE55 commented Jan 9, 2024

Does the following code violate the readonly constraint of arr_of_cells: &[Cell<i32>]?

 let mut arr = [0; 10];
 let cell_arr = Cell::from_mut(&mut arr);
 let arr_of_cells = cell_arr.as_array_of_cells();
 cell_arr.set([1; 10]);
// arr_of_cells was changed.
 dbg!(arr_of_cells);

@oconnor663
Copy link
Contributor Author

@TOETOE55 Miri is ok with it at least. I could be totally wrong, but I think Miri sees these things in terms of bytes, and all the bytes in this case are covered by UnsafeCell, and that might be why it's ok.

@scottmcm what needs to be done to move to FCP?

@TOETOE55
Copy link

TOETOE55 commented Jan 10, 2024

The following code also passes Miri's checks
, but whether this should be UB is questionable. If I remember correctly, the current rule for the checks is that &T is readonly only when T does not contain UnsafeCell.

use core::cell::Cell;

fn main() {
    let mut opt = Some(1);
    let cell_opt = Cell::from_mut(&mut opt);
    let opt_cell = unsound(&cell_opt);
    dbg!(opt_cell.as_ref().unwrap());
    cell_opt.set(None);
    dbg!(opt_cell);
}

fn unsound<T>(opt: &Cell<Option<T>>) -> &Option<Cell<T>> {
    unsafe { &*(opt as *const Cell<Option<T>> as *const Option<Cell<T>>) }
}

@RalfJung
Copy link
Member

RalfJung commented Oct 5, 2024

@rust-lang/libs-api do you see any blockers for stabilizing this?

Does the following code violate the readonly constraint of arr_of_cells: &[Cell]?

No, this is fine -- you're only mutating things inside an UnsafeCell.

The following code also passes Miri's checks, but whether this should be UB is questionable.

Correct, but similar code can already be written with as_slice_of_cells. So, that's not really related to this tracking issue at all.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 5, 2024
make Cell unstably const

Now that we can do interior mutability in `const`, most of the Cell API can be `const fn`. :)  The main exception is `set`, because it drops the old value. So from const context one has to use `replace`, which delegates the responsibility for dropping to the caller.

Tracking issue: rust-lang#131283

`as_array_of_cells` is itself still unstable to I added the const-ness to the feature gate for that function and not to `const_cell`, Cc rust-lang#88248.

r? libs-api
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 5, 2024
Rollup merge of rust-lang#131281 - RalfJung:const-cell, r=Amanieu

make Cell unstably const

Now that we can do interior mutability in `const`, most of the Cell API can be `const fn`. :)  The main exception is `set`, because it drops the old value. So from const context one has to use `replace`, which delegates the responsibility for dropping to the caller.

Tracking issue: rust-lang#131283

`as_array_of_cells` is itself still unstable to I added the const-ness to the feature gate for that function and not to `const_cell`, Cc rust-lang#88248.

r? libs-api
@Amanieu
Copy link
Member

Amanieu commented Oct 7, 2024

Should this be named transpose instead for consistency with the same method on MaybeUninit<[N; T]> and [MaybeUninit<T>; N]? It would also be an opportunity to introduce the reverse operation also named transpose.

@RalfJung
Copy link
Member

RalfJung commented Oct 7, 2024

I don't like transpose as a name, it says very little about what the operation actually does. as_slice_of_cells/as_array_of_cells is nicely explicit, which I think is a good thing.

@TOETOE55
Copy link

TOETOE55 commented Oct 7, 2024

Does the following code violate the readonly constraint of arr_of_cells: &[Cell]?

No, this is fine -- you're only mutating things inside an UnsafeCell.

What confuses me is that this code modifies arr_of_cells: &[Cell] through cell_arr: Cell<[i32; N]>, yet arr_of_cells itself is not in an UnsafeCell, only its elements are.

@RalfJung
Copy link
Member

RalfJung commented Oct 7, 2024

I'm afraid I cannot parse that sentence... " the arr_of_cell is not " is not what?

@TOETOE55
Copy link

TOETOE55 commented Oct 7, 2024

I'm afraid I cannot parse that sentence... " the arr_of_cell is not " is not what?

sorry, my English is not very good, the comment has been edited.

@RalfJung
Copy link
Member

RalfJung commented Oct 7, 2024

yet arr_of_cells itself is not in an UnsafeCell, only its elements are.

Rust doesn't have an object model where there exists something like an "array object" that "contains" its fields or so. As far as Rust is concerned, an array is exactly the same thing as the collection of all its fields. So if call fields are fully covered by UnsafeCell, then so is the entire array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-array Area: `[T; N]` C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants