Add basic slice support#152
Conversation
28732a1 to
40d296d
Compare
There was a problem hiding this comment.
Pull request overview
Adds first-class support for Rust slices ([T]) in Thrust’s modeling + MIR analysis pipeline by (a) mapping slices to the existing Seq model and (b) recognizing slice-specific MIR lowering patterns (length via PtrMetadata, indexing via bounds-check + PlaceElem::Index) and reifying them back into standard library operations so existing call/type machinery applies.
Changes:
- Model
[T]asmodel::Seq<T>and add extern specs for common slice operations (len,get/get_mut,first/last,Index/IndexMut) instd.rs. - Add a MIR reconstruction pass that rewrites lowered slice indexing back into
Index::index/IndexMut::index_mutcalls and hook it into analysis. - Add paired UI pass/fail tests plus compiler-backed unit tests for the reconstruction behavior; enable library unit tests.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ui/pass/slice_methods.rs | UI coverage for basic shared slice methods and trait-based Index::index. |
| tests/ui/pass/slice_methods_mut.rs | UI coverage for get_mut and subsequent readback. |
| tests/ui/pass/slice_last_mut.rs | UI coverage for last_mut + last consistency. |
| tests/ui/pass/slice_index.rs | UI coverage for len() and slice[idx] desugaring. |
| tests/ui/pass/slice_index_mut.rs | UI coverage for slice[idx] += 1 via IndexMut. |
| tests/ui/pass/slice_first_mut.rs | UI coverage for first_mut + first consistency. |
| tests/ui/fail/slice_methods.rs | Negative UI coverage for incorrect last() expectation. |
| tests/ui/fail/slice_methods_mut.rs | Negative UI coverage for incorrect post-mutation value. |
| tests/ui/fail/slice_last_mut.rs | Negative UI coverage for incorrect last() expectation after mutation. |
| tests/ui/fail/slice_index.rs | Negative UI coverage for out-of-bounds indexing. |
| tests/ui/fail/slice_index_mut.rs | Negative UI coverage for incorrect IndexMut result expectation. |
| tests/ui/fail/slice_first_mut.rs | Negative UI coverage for incorrect first() expectation after mutation. |
| std.rs | Adds [T] model + extern specs for slice ops used by analysis/reconstruction. |
| src/lib.rs | Adds rustc_driver import behind cfg(test) for compiler-backed unit tests. |
| src/analyze/reconstruct_slice_indexing.rs | New pass to reconstruct slice indexing calls from optimized MIR patterns. |
| src/analyze/reconstruct_slice_indexing/tests.rs | New compiler-backed unit tests validating reconstructed MIR call shapes. |
| src/analyze/local_def.rs | Runs the reconstruction pass as part of analysis pipeline. |
| src/analyze/basic_block.rs | Recognizes slice len() lowered to PtrMetadata and types it as a call. |
| src/analyze.rs | Registers the new analysis module and adds an internal fn_operand helper. |
| Cargo.toml | Enables lib unit tests so the new reconstruction tests can run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40d296d9e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (index < slice.1 && result == Some(&slice.0[index])) | ||
| || (slice.1 <= index && result == None) | ||
| )] | ||
| fn _extern_spec_slice_get<T>(slice: &[T], index: usize) -> Option<&T> |
There was a problem hiding this comment.
Gate slice get specs to usize indices
Because extern specs are registered by the target DefId, this contract for the generic <[T]>::get method is reused for every SliceIndex instantiation, not just usize. A valid call like slice.get(0..2) then tries to type a range argument against this usize/index-element contract and can panic during subtyping instead of either falling back or using range-slice semantics; the mirrored get_mut spec below has the same issue.
Useful? React with 👍 / 👎.
74906dd to
4e5abf2
Compare
Summary
[T]with the existingSeqmodellen,get,get_mut,first,first_mut,last,last_mut,Index, andIndexMutDesign
Knowledge that slices are modeled as
Seqremains instd.rs. The analyzer only maps slice-specific MIR operations back to standard slice operations and applies their registered function types; it never constructsSeqterms for slices.Slice length and indexing use different integration points because they have different MIR shapes:
PtrMetadata(slice)rvalue. It already has a local input/output boundary, so assignment analysis can invoketype_callwith[T]::lenand bind the scalar result directly. Reconstructing a real call here would require splitting the basic block and inserting a new call terminator without improving borrow or prophecy handling.PlaceElem::Indexuse. Mutable indexing also produces a reference whose prophecy and environment transition should use the normal call machinery. The reconstruction pass therefore recognizes this canonical MIR shape, replaces it with anIndex::indexorIndexMut::index_mutcall, and rewrites the indexed place to dereference the returned reference.Reconstruction is an implementation choice rather than a semantic requirement: direct indexed-place analysis could also remain model-independent, but it would require making place elaboration effectful and threading call, borrow, and prophecy handling through those paths.
Validation
cargo check