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

Check transmute restrictions in the crate where it is used #12898

Closed
brson opened this issue Mar 14, 2014 · 8 comments
Closed

Check transmute restrictions in the crate where it is used #12898

brson opened this issue Mar 14, 2014 · 8 comments
Milestone

Comments

@brson
Copy link
Contributor

brson commented Mar 14, 2014

The transmute function is peculiar in that it has extra restrictions on its type parameters that are not checked until translation time. This can result in surprises for downstream users of code that uses transmute. Instead we should type check its instantiation when called or its value is taken. Doing this will require at least one new restriction.

During type check:

  • Validate that generic types are only transmuted through pointers
    • this ensures that all uses can be validated in the context of the local trait
    • todo: we have fat and thin pointers, so this would seem insufficient
  • Validate that both types are Sized or the same type
  • Record all pairs of transmuted types

During translation (of the crate where transmute is used):

  • Check that all instantiations used types with the same type
  • Warn if any instantiations used types with different alignment

@nikomatsakis Does this sound right? What about the fat/thin pointer issue?

Nominating.

@nikomatsakis
Copy link
Contributor

I don't know why you say "record all pairs of transmuted types".

I don't know whether it makes sense to warn -- certainly this should be lintable.

Regarding the thin/fat pointer issue, we'd have to report an error (during type check) if you transmute from &T to &U and either T or U is not Sized.

@nikomatsakis
Copy link
Contributor

To put another way, I think the idea was to have type check validate that trans will succeed. Sometimes, such as a transmute from T to U (where T and U are typed parameters) or a transmute from &T to &U where T or U are sized, the type checker has to be conservative.

@pnkfelix
Copy link
Member

P-backcompat-lang, 1.0.

@nikomatsakis
Copy link
Contributor

It'd probably be useful to survey existing uses of transmute and describe them here. I'd be interested to know how they breakdown!

@nikomatsakis
Copy link
Contributor

I just had an interesting thought regarding const-casts: I'd really like to prevent people from transmuting &T to &mut T (except via Unsafe<T> -- I'm not quite sure how to allow Unsafe<T> to do it while forbidding everyone else). The reason being that this result is undefined behavior. It's easy enough to prevent &T from being transmuted to &mut U as part of this change, and I think we should do that.

However, it seems clear that's not enough. For example, you could always have a transmute from &T to &U and -- somewhere in types T and U -- you might convert a & to &mut. But it occurs to me now that even if you did transmute it, you could never use the &mut value that is within U to mutate, since it itself is still in an aliased location. This is still not an actual guarantee of avoiding undefined behavior -- you could transmute from *mut T to *mut U (with, say, T=&int and U=&mut int) and skin the cat that way -- but it's closer than I thought.

The only way to really check for this sort of const cast would be either (a) add a (presumably special) TransmutableTo trait or (b) keep the current "check during trans" approach.

In the course of writing this message, though, I've started to wonder if preventing trans errors is good enough, or if we shouldn't strive to actually prevent const casts altogether. In particular, I think that we still can't really guarantee that trans will succeed for arbitrary types, at least not once associated types come into play -- I imagine it'd be possible to generate a monomorphization expansion with certain types that never terminates. (Generating foo::<A> then generates foo::<A::T>, where A::T (in some cases) is defined as ~A, such that we wind up monomorphizing foo::<A>, then foo::<~A>, then foo::<~~A>, etc). This is technically different, I guess (nontermination vs error) but to the end user, it's maybe not so different.

@nikomatsakis
Copy link
Contributor

Caveat to last paragraph: we don't, of course, have associated types, so maybe there's some reason something like that last example can't be done.

@emberian
Copy link
Member

@nikomatsakis Unsafe<T> doesn't actually do that transmute. It returns a *mut which is the only valid type to return, once we start emitting noalias on &mut T.

@nikomatsakis
Copy link
Contributor

@cmr it starts with an &self, converts to *self, and then transmutes to *mut self. This last transmute is the one I was talking about.

alexcrichton added a commit to alexcrichton/rust that referenced this issue May 10, 2014
This commit revisits the `cast` module in libcore and libstd, and scrutinizes
all functions inside of it. The result was to remove the `cast` module entirely,
folding all functionality into the `mem` module. Specifically, this is the fate
of each function in the `cast` module.

* transmute - This function was moved to `mem`, but it is now marked as
              #[unstable]. This is due to planned changes to the `transmute`
              function and how it can be invoked (see the #[unstable] comment).
              For more information, see RFC 5 and rust-lang#12898

* transmute_copy - This function was moved to `mem`, with clarification that is
                   is not an error to invoke it with T/U that are different
                   sizes, but rather that it is strongly discouraged. This
                   function is now #[stable]

* forget - This function was moved to `mem` and marked #[stable]

* bump_box_refcount - This function was removed due to the deprecation of
                      managed boxes as well as its questionable utility.

* transmute_mut - This function was previously deprecated, and removed as part
                  of this commit.

* transmute_mut_unsafe - This function doesn't serve much of a purpose when it
                         can be achieved with an `as` in safe code, so it was
                         removed.

* transmute_lifetime - This function was removed because it is likely a strong
                       indication that code is incorrect in the first place.

* transmute_mut_lifetime - This function was removed for the same reasons as
                           `transmute_lifetime`

* copy_lifetime - This function was moved to `mem`, but it is marked
                  `#[unstable]` now due to the likelihood of being removed in
                  the future if it is found to not be very useful.

* copy_mut_lifetime - This function was also moved to `mem`, but had the same
                      treatment as `copy_lifetime`.

* copy_lifetime_vec - This function was removed because it is not used today,
                      and its existence is not necessary with DST
                      (copy_lifetime will suffice).

In summary, the cast module was stripped down to these functions, and then the
functions were moved to the `mem` module.

    transmute - #[unstable]
    transmute_copy - #[stable]
    forget - #[stable]
    copy_lifetime - #[unstable]
    copy_mut_lifetime - #[unstable]

[breaking-change]
bors added a commit that referenced this issue May 10, 2014
This commit revisits the `cast` module in libcore and libstd, and scrutinizes
all functions inside of it. The result was to remove the `cast` module entirely,
folding all functionality into the `mem` module. Specifically, this is the fate
of each function in the `cast` module.

* transmute - This function was moved to `mem`, but it is now marked as
              #[unstable]. This is due to planned changes to the `transmute`
              function and how it can be invoked (see the #[unstable] comment).
              For more information, see RFC 5 and #12898

* transmute_copy - This function was moved to `mem`, with clarification that is
                   is not an error to invoke it with T/U that are different
                   sizes, but rather that it is strongly discouraged. This
                   function is now #[stable]

* forget - This function was moved to `mem` and marked #[stable]

* bump_box_refcount - This function was removed due to the deprecation of
                      managed boxes as well as its questionable utility.

* transmute_mut - This function was previously deprecated, and removed as part
                  of this commit.

* transmute_mut_unsafe - This function doesn't serve much of a purpose when it
                         can be achieved with an `as` in safe code, so it was
                         removed.

* transmute_lifetime - This function was removed because it is likely a strong
                       indication that code is incorrect in the first place.

* transmute_mut_lifetime - This function was removed for the same reasons as
                           `transmute_lifetime`

* copy_lifetime - This function was moved to `mem`, but it is marked
                  `#[unstable]` now due to the likelihood of being removed in
                  the future if it is found to not be very useful.

* copy_mut_lifetime - This function was also moved to `mem`, but had the same
                      treatment as `copy_lifetime`.

* copy_lifetime_vec - This function was removed because it is not used today,
                      and its existence is not necessary with DST
                      (copy_lifetime will suffice).

In summary, the cast module was stripped down to these functions, and then the
functions were moved to the `mem` module.

    transmute - #[unstable]
    transmute_copy - #[stable]
    forget - #[stable]
    copy_lifetime - #[unstable]
    copy_mut_lifetime - #[unstable]
alexcrichton added a commit to alexcrichton/rust that referenced this issue May 11, 2014
This commit revisits the `cast` module in libcore and libstd, and scrutinizes
all functions inside of it. The result was to remove the `cast` module entirely,
folding all functionality into the `mem` module. Specifically, this is the fate
of each function in the `cast` module.

* transmute - This function was moved to `mem`, but it is now marked as
              #[unstable]. This is due to planned changes to the `transmute`
              function and how it can be invoked (see the #[unstable] comment).
              For more information, see RFC 5 and rust-lang#12898

* transmute_copy - This function was moved to `mem`, with clarification that is
                   is not an error to invoke it with T/U that are different
                   sizes, but rather that it is strongly discouraged. This
                   function is now #[stable]

* forget - This function was moved to `mem` and marked #[stable]

* bump_box_refcount - This function was removed due to the deprecation of
                      managed boxes as well as its questionable utility.

* transmute_mut - This function was previously deprecated, and removed as part
                  of this commit.

* transmute_mut_unsafe - This function doesn't serve much of a purpose when it
                         can be achieved with an `as` in safe code, so it was
                         removed.

* transmute_lifetime - This function was removed because it is likely a strong
                       indication that code is incorrect in the first place.

* transmute_mut_lifetime - This function was removed for the same reasons as
                           `transmute_lifetime`

* copy_lifetime - This function was moved to `mem`, but it is marked
                  `#[unstable]` now due to the likelihood of being removed in
                  the future if it is found to not be very useful.

* copy_mut_lifetime - This function was also moved to `mem`, but had the same
                      treatment as `copy_lifetime`.

* copy_lifetime_vec - This function was removed because it is not used today,
                      and its existence is not necessary with DST
                      (copy_lifetime will suffice).

In summary, the cast module was stripped down to these functions, and then the
functions were moved to the `mem` module.

    transmute - #[unstable]
    transmute_copy - #[stable]
    forget - #[stable]
    copy_lifetime - #[unstable]
    copy_mut_lifetime - #[unstable]

[breaking-change]
bors added a commit that referenced this issue May 11, 2014
This commit revisits the `cast` module in libcore and libstd, and scrutinizes
all functions inside of it. The result was to remove the `cast` module entirely,
folding all functionality into the `mem` module. Specifically, this is the fate
of each function in the `cast` module.

* transmute - This function was moved to `mem`, but it is now marked as
              #[unstable]. This is due to planned changes to the `transmute`
              function and how it can be invoked (see the #[unstable] comment).
              For more information, see RFC 5 and #12898

* transmute_copy - This function was moved to `mem`, with clarification that is
                   is not an error to invoke it with T/U that are different
                   sizes, but rather that it is strongly discouraged. This
                   function is now #[stable]

* forget - This function was moved to `mem` and marked #[stable]

* bump_box_refcount - This function was removed due to the deprecation of
                      managed boxes as well as its questionable utility.

* transmute_mut - This function was previously deprecated, and removed as part
                  of this commit.

* transmute_mut_unsafe - This function doesn't serve much of a purpose when it
                         can be achieved with an `as` in safe code, so it was
                         removed.

* transmute_lifetime - This function was removed because it is likely a strong
                       indication that code is incorrect in the first place.

* transmute_mut_lifetime - This function was removed for the same reasons as
                           `transmute_lifetime`

* copy_lifetime - This function was moved to `mem`, but it is marked
                  `#[unstable]` now due to the likelihood of being removed in
                  the future if it is found to not be very useful.

* copy_mut_lifetime - This function was also moved to `mem`, but had the same
                      treatment as `copy_lifetime`.

* copy_lifetime_vec - This function was removed because it is not used today,
                      and its existence is not necessary with DST
                      (copy_lifetime will suffice).

In summary, the cast module was stripped down to these functions, and then the
functions were moved to the `mem` module.

    transmute - #[unstable]
    transmute_copy - #[stable]
    forget - #[stable]
    copy_lifetime - #[unstable]
    copy_mut_lifetime - #[unstable]
pcwalton added a commit to pcwalton/rust that referenced this issue Jun 13, 2014
only known post-monomorphization, and report `transmute` errors before
the code is generated for that `transmute`.

This can break code that looked like:

    unsafe fn f<T>(x: T) {
        let y: int = transmute(x);
    }

Change such code to take a type parameter that has the same size as the
type being transmuted to.

Closes rust-lang#12898.

[breaking-change]
matthiaskrgr pushed a commit to matthiaskrgr/rust that referenced this issue Aug 2, 2022
fix: Fix pattern completions adding unnecessary braces

Fixes rust-lang/rust-analyzer#12852
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.

4 participants