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

Containing transmutes on slices and other representations. #6790

Closed
Kimundi opened this issue May 28, 2013 · 3 comments
Closed

Containing transmutes on slices and other representations. #6790

Kimundi opened this issue May 28, 2013 · 3 comments

Comments

@Kimundi
Copy link
Member

Kimundi commented May 28, 2013

Right now a few places in the code base make assumptions about the internal representations of things like &[T] or other types that stand for internal representation details, mostly to implement low-level operations in unsafe blocks. For example you could see code like this:

pub fn my_slice<'r,T>(v: &'r [T], start: uint, end: uint) -> &'r [T] {
    assert!(start <= end);
    assert!(end <= v.len());
    unsafe {
        let mut (buf, len): (*T, uint) = transmute(v);
        buf += start;
        len = end - start;
        transmute(buf, len)
    }
}

This is fine as long as the internal representation of those things doesn't change, but will wreak random havoc if something changes (for example as result of the implementation of the Dynamic types proposal, or a change to the way &str handles its hidden trailing byte)

To improve on this situation, I think the following needs to be done for all build-in 'special' types like &[T], &str, char etc:

  • Have a central place in std::unstable for the underlying representation. It would contain things like struct SliceRep<T> { buf: *T, len:uint } or struct CharRep { v: uint }.
  • Have functions to safely cast a type to its repr, to unsafely cast a repr to its type, and if possible functions to safely cast a repr to it's type provided all necessary invariants are unviolated.
  • Implement a lint warning for calls of transmute-like functions from/to those types. All low level work and casts to representations would then have to happen through the functions in the corresponding modules, where the warnings get explicitly disabled with an attribute.

The canonical warn-free way to write my_slice would then become something like this:

pub fn my_slice<'r,T>(v: &'r [T], start: uint, end: uint) -> &'r [T] {
    assert!(start <= end);
    assert!(end <= v.len());
    let mut repr = v.to_internal_repr();
    unsafe {
        repr.buf += start;
        repr.len = end - start;
        repr.to_type()
    }
}
@nikomatsakis
Copy link
Contributor

+1. Note that the first part (add abstractions) can be done even without the lint, and then just enforced via code review.

bors added a commit that referenced this issue Jul 27, 2013
This moves the raw struct layout of closures, vectors, boxes, and strings into a
new `unstable::raw` module. This is meant to be a centralized location to find
information for the layout of these values.

As safe method, `unwrap`, is provided to convert a rust value to its raw
representation. Unsafe methods to convert back are not provided because they are
rarely used and too numerous to write an implementation for each (not much of a
common pattern).

This is progress on #6790. I tried to get a nice interface for a trait to implement in the raw module, but I was unable to come up with one. The hard part is that there are so many different directions to go from one way to another that it's difficult to find a pattern to follow to implement a trait with. Someone else might have some better luck though.
@huonw
Copy link
Member

huonw commented Sep 30, 2013

Triage: #7986 gave us std::unstable::raw which gives some struct definitions and provides the .repr() method for converting &[T], ~str etc, to them.

A quick glance over the results of git grep transmute indicates we don't have (m)any slice transmutations that aren't using std::unstable::raw left after that PR. We still don't have a lint, and the current cast-to-slice method is something like:

            unsafe {
                cast::transmute(Slice {
                    data: ptr::mut_offset(p, start as int) as *T,
                    len: (end - start) * sys::nonzero_size_of::<T>()
                })
            }

which is far batter than it was (named fields), but not quite as self-contained as a single function.

(Also, I imagine that many reviewers don't know about std::unstable::raw and so won't know that there is a better way; do we have a "reviewers checklist" to which we can add tidbits like this?)

@alexcrichton
Copy link
Member

Closing, I think that std::raw satisfies much of the original intent of this issue. Reviewers are quite often cracking down on transmutes as well, so I think we're in a good situation.

flip1995 pushed a commit to flip1995/rust that referenced this issue Mar 11, 2021
or_fun_call: fix suggestion for `or_insert(vec![])`

fixes rust-lang#6748
changelog: or_fun_call: fix suggestion for `or_insert(vec![])` on `std::collections::hash_map::Entry` or `std::collections::btree_map::Entry`
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

No branches or pull requests

4 participants