Skip to content

Typeck: Disallow scalar casts to bare_fn. #9788

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

Merged
merged 1 commit into from
Nov 14, 2013
Merged

Typeck: Disallow scalar casts to bare_fn. #9788

merged 1 commit into from
Nov 14, 2013

Conversation

geoffhill-zz
Copy link
Contributor

Bare functions are another example of a scalar but non-numeric
type (like char) that should be handled separately in casts.
This disallows expressions like 0 as extern "Rust" fn() -> int;.

It might be advantageous to allow casts between bare functions
and raw pointers in unsafe code in the future, to pass function
pointers between Rust and C.

Closes #8728

Bare functions are another example of a scalar but non-numeric
type (like char) that should be handled separately in casts.
This disallows expressions like `0 as extern "Rust" fn() -> int;`.

It might be advantageous to allow casts between bare functions
and raw pointers in unsafe code in the future, to pass function
pointers between Rust and C.

Closes #8728
@alexcrichton
Copy link
Member

I'm surprised this works currently, nice catch!

I don't personally know exactly what would have caused this in the first place, but I know that @nikomatsakis was working with extern fns recently and may be able to shed some light on this.

@nikomatsakis
Copy link
Contributor

Hmm, I think the real problem is that bare fns and raw pointers should not be considered scalars. I suspect that the reason they are considered scalars, however, is to enable other kinds of casts (e.g., uint to *int). I know that this is one way to get null pointers in static constants, where transmute is disallowed, though I think that these days we could also use Option<*int> etc instead. In general I'd prefer to fix the "is_scalar" categorization than to patch up the typeck side of the equation -- but overall I'm not precisely sure which casts should be legal. @jdm -- you've done a lot of the work on static constants, does this ring any bells?

@jdm
Copy link
Contributor

jdm commented Oct 10, 2013

If Option<*T> is equivalent to *T, it definitely seems preferable to remove the ability to ability to cast integer types to unsafe pointers and use Option in FFI definitions instead. My changes allowed raw pointer to raw pointer casts, integer to raw pointer, and region pointer to raw pointer. The latter is important for creating static C string values from vectors, while the former is important for being able to convert pointers to constant Rust data into types that can be passed to external C libraries.

@geoffhill-zz
Copy link
Contributor Author

The current code path treats quite a few of the scalars as exceptional cases: chars, C-like enums, nil, raw pointers, and now bare functions... A simpler code path that more closely matches the docs shouldn't involve scalars at all.

Allow the following casts:

  • NumberValued => Numeric
  • Integers, region pointers, and raw pointers => raw pointers
  • u8 => char

where Integer = ( ints, uints )
and Numeric = Integers ++ ( floats, c_like_enums )
and NumberValued = Numerics ++ ( bools, chars )

@brson
Copy link
Contributor

brson commented Oct 26, 2013

@nikomatsakis Do you agree with @geoffhill's assessment? Do we know how to fix this?

@nikomatsakis
Copy link
Contributor

Based on what @jdm said, I'd be inclined to push for fewer legal casts. We can always add more later. Therefore, I think we can even support fewer casts than what @geoffhill suggested.

I think I would permit:

  • Numbers => Numbers
  • Region Pointer => Unsafe Pointer (we'll even do this implicitly)
  • u8 => char
  • C-like-enum => int

But I would not support:

  • uint => raw pointer, raw pointer => uint
  • raw pointer => raw pointer

To cover the last two cases, we can offer variations on transmute(). In general, it'd be preferable to use Option<*T>, which also means people can just write None in constants.

I have to admit to some trepidation though. I don't want to make using *T more painful, really, and pushing people to Option<*T> might be annoying.

@pcwalton
Copy link
Contributor

I agree. I'm inclined to r+ this. Does anyone disagree?

@geoffhill-zz
Copy link
Contributor Author

I like the casts that @nikomatsakis proposed, effectively separating the safe casts from casts only really needed for interfacing with C.

Do I understand it right then that we also want to disallow bool => Number and char => Number casts? We should provide a function for the latter if that's the case. Also what about bare_fn => Raw Pointer?

Seems the next step here is to remove the Integer <=> Raw Pointer and Raw Pointer <=> Raw Pointers casts. This will show us where the real use cases are, and we can provide transmute() variants for them. After that, a complete test suite to detail all the allowed casts, and update to docs and cast code to match.

@nikomatsakis
Copy link
Contributor

On Wed, Oct 30, 2013 at 12:49:55PM -0700, Geoff Hill wrote:

Do I understand it right then that we also want to disallow bool => Number and char => Number casts?

I'd probably keep those.

Also what about bare_fn => Raw Pointer?

This seems like it falls into the same category as raw pointer <=> raw pointer:
unsafe changes with respect to what the pointer points at.

Seems the next step here is to remove the Integer <=> Raw Pointer
and Raw Pointer <=> Raw Pointers casts. This will show us where the
real use cases are, and we can provide transmute() variants for
them. After that, a complete test suite to detail all the allowed
casts, and update to docs and cast code to match.

Sounds good to me!

@jdm
Copy link
Contributor

jdm commented Oct 31, 2013

I'll note that raw pointer casts can be useful for statics where transmute isn't available. Granted, any time you have a cast in a static, you could instead call transmute when referencing it in a non-static expression. It does increase the spread of unsafe code, however.

@nikomatsakis
Copy link
Contributor

On Wed, Oct 30, 2013 at 09:31:49PM -0700, Josh Matthews wrote:

I'll note that raw pointer casts can be useful for statics where
transmute isn't available. Granted, any time you have a cast in a
static, you could instead call transmute when referencing it in a
non-static expression. It does increase the spread of unsafe code,
however.

I guess we have to be sure to permit some way to go from a "foo"
string to a char*. I'm not sure that the restricted casting rules I
mentioned included that.

@catamorphism
Copy link
Contributor

@pcwalton It sounds like no one disagrees and this is still mergeable -- any reason not to r+ it?

bors added a commit that referenced this pull request Nov 14, 2013
Bare functions are another example of a scalar but non-numeric
type (like char) that should be handled separately in casts.
This disallows expressions like `0 as extern "Rust" fn() -> int;`.

It might be advantageous to allow casts between bare functions
and raw pointers in unsafe code in the future, to pass function
pointers between Rust and C.

Closes #8728
@bors bors closed this Nov 14, 2013
@bors bors merged commit e538c95 into rust-lang:master Nov 14, 2013
@geoffhill-zz geoffhill-zz deleted the bare-fn-cast branch May 8, 2014 21:11
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.

It's possible to cast 0 as extern fn() in safe code
9 participants