Skip to content

Fixes Miri error, Optimizes retain implementation, Adds benchmarks - #78

Open
fereidani wants to merge 3 commits into
mozilla:mainfrom
fereidani:main
Open

Fixes Miri error, Optimizes retain implementation, Adds benchmarks#78
fereidani wants to merge 3 commits into
mozilla:mainfrom
fereidani:main

Conversation

@fereidani

Copy link
Copy Markdown
Contributor

No description provided.

@emilio

emilio commented Apr 8, 2026

Copy link
Copy Markdown
Collaborator

Can you post benchmark results of before and after? Skeptic about the early "return for optimization" and some similar changes which feel drive-by?

emilio pushed a commit that referenced this pull request Apr 8, 2026
Error in question:

    ---- src/lib.rs - ThinVec<T>::extract_if (line 1636) stdout ----
    Test executable failed (exit status: 1).

    stderr:
    error: Undefined Behavior: trying to retag from <1422> for Unique permission at alloc211[0x10], but that tag does not exist in the borrow stack for this location
        --> src/lib.rs:2999:25
         |
    2999 |                 let v = slice::from_raw_parts_mut(self.vec.as_mut_ptr(), self.old_len);
         |                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this error occurs as part of retag at alloc211[0x10..0x40]
         |
         = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
         = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
    help: <1422> would have been created here, but this is a zero-size retag ([0x10..0x10]) so the tag in question does not exist anywhere
        --> src/lib.rs:2999:51
         |
    2999 |                 let v = slice::from_raw_parts_mut(self.vec.as_mut_ptr(), self.old_len);
         |                                                   ^^^^^^^^^^^^^^^^^^^^^

Cherry-picked from #78
@fereidani

Copy link
Copy Markdown
Contributor Author

Hey, that's an LLVM trick, the Rust standard library uses it quite often too. This new algorithm is already in there; my PR got merged a few months ago to rust itself.
You can check the Rust standard library directly. The benchmarks are in my branch if you want to run them yourself and see the results.

@emilio

emilio commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

@fereidani could you link to that PR? Thanks.

@fereidani

Copy link
Copy Markdown
Contributor Author

Sure: rust-lang/rust#149784

@fereidani

Copy link
Copy Markdown
Contributor Author

That's unsafe implementation of the same algorithm.

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.

2 participants