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

remove the concept of explicit casting as it currently exists #1061

Closed
andrewrk opened this issue Jun 6, 2018 · 8 comments
Closed

remove the concept of explicit casting as it currently exists #1061

andrewrk opened this issue Jun 6, 2018 · 8 comments
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Jun 6, 2018

Right now there are various builtin functions which can do cast operations, for example:

  • @ptrCast
  • @bitCast
  • @truncate
  • @intToPtr

These have varying levels of unsafety associated with them, and they have very specific, clearly communicated intentions. They're here to stay.

We also have another concept of implicit casting vs explicit casting. For example, implicit casts:

var a: u32 = 1234; // implicitly cast a comptime_int to a u32
var b: u8 = a; // implicitly cast a u32 to a u8.

Implicit casts are the preferred way to convert between types because:

  • They are always safe
  • The compiler knows how to perform them at comptime

In the above code, there would be a compile error because u8 does not cover the range of u32.

Explicit casts:

var a = u32(1234); // explicitly cast a comptime_int to a u32
var b = u8(a); // explicitly cast a u32 to a u8.

Explicit casts are when a type is invoked as a function call. In many cases this has the same behavior as implicit casts, however, sometimes they "force" a conversion. For example, in the above code, the explicit cast overrides the compile error that an implicit cast would give and instead adds a runtime safety check, which in this case will crash the program when run in debug mode.

I propose to remove this "explicit cast" override, instead adding builtin functions for the various kinds of casting that one might want to do. For example, the above code would use a new builtin called @intShorten. Using a type as a function call will then serve as only a syntax convenience to perform an "implicit cast".

An optional addendum to this proposal would be to remove the syntax completely, forcing zig programmers to utilize variable declarations in order to achieve implicit casts.

@andrewrk andrewrk added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Jun 6, 2018
@andrewrk andrewrk added this to the 0.3.0 milestone Jun 6, 2018
@BraedonWooding
Copy link
Contributor

BraedonWooding commented Jun 6, 2018

I don't agree with removing it completely, since there are multiple cases where it would just add verbosity and some cases you need an explicit cast (unicode is a use case for this, and the std lib has quite a few other cases where it is used), especially with the introduction of 'arbitrary' length integers the use of explicit casts I feel designates extent clear enough. However I'm all for a 'shorten' cast.

I don't see a use case for floats or other types though, so I'm all for removing those kind of explicit casts and just having a builtin for shortening ints.

Edit: just to clarify, I'm all for removing explicit casts and introducing intShorten :), just not removing intShorten (or rather not introducing it) and removing explicit casts.

@thejoshwolfe
Copy link
Sponsor Contributor

the std lib has quite a few other cases where it is used

Can you point to specific examples?

@BraedonWooding
Copy link
Contributor

BraedonWooding commented Jun 6, 2018

Zig Fmt Errol/index.zig: Line: 581

...
const a = u32(value / kTen16); // 1 to 1844
value %= kTen16;
        if (a < 10) {
            buffer[buf_index] = '0' + u8(a); // THIS LINE: 'a' is a 32 bit integer
            buf_index += 1;
        } else if (a < 100) {
            const i: u32 = a << 1;
            buffer[buf_index] = c_digits_lut[i];
            buf_index += 1;
            buffer[buf_index] = c_digits_lut[i + 1];
            buf_index += 1;
        } else if (a < 1000) {
            buffer[buf_index] = '0' + u8(a / 100); // THIS LINE: 'a' is a 32 bit integer
            buf_index += 1;

            const i: u32 = (a % 100) << 1; // needed as 32 here
            buffer[buf_index] = c_digits_lut[i];
            buf_index += 1;
            buffer[buf_index] = c_digits_lut[i + 1];
            buf_index += 1;
...

This was one I found by searching for it, I also have come across cases; I'm sure that @Hejsil (maybe) also has come across cases with handling a rom randomizer. Really any program low level benefits from explicit casting. Removing it effects embedded development I feel (from my somewhat limited experience sure).

@Hejsil
Copy link
Sponsor Contributor

Hejsil commented Jun 6, 2018

Actually, my code has very few explicit casts. What I use most is slice widening casts (([]const T)(some_bytes)), which could probably be implemented in userland with @ptrCast and length division.

I think removing the current explicit casts in favor of builtins has a couple of advantages:

  • Only one way: Currently I juggle between var: usize = 0; and var = size(0);. The second is a little easier to type, but I'm a bad person and not consistent, so I have both in my codebase :(
    Also, as far as I know, nearly all required explicit casts are convertions, just like @ptrCast, @ptrToInt and so on. It would probably be more consistent to either have explicit casts or builtins for these conversions, and not both.
  • Favor reading code over writing code.: In normal, every day code, I think that the difference between @intShorten(u8, v) and u8(v) are null. However, in generic code, @intShorten(T, v) wins over T(v).
  • Lastly, cast heavy code it probably unreadable regardless of wether it uses builtins or the current casts, so we might as well discorage it my making casts require a little more effort :)

@raulgrell
Copy link
Contributor

I'm open to this code being silly, but this my most significant use of the feature, casting back and forth between ints and floats with a single syntax:

pub fn Vec2T(comptime T: type) type {
    return packed struct {
        x: T,
        y: T,

        const Self = this;

        pub fn cast(self: &const Self, comptime C: type) Vec2T(C) {
            return Vec2T(C) { .x = C(self.x), .y = C(self.y) };
        }
    };
}

pub fn main() void {
    const a = Vec2T(f32) { .x = 1.5, .y = -4.5 };
    const aa = a.cast(i32);
    const b = Vec2T(i32) { .x = 1, .y = 42 };
    const bb = b.cast(f32);
    debug.warn("{}, {}\n", aa.x, aa.y);
    debug.warn("{}, {}\n", bb.x, bb.y);
}

@BraedonWooding
Copy link
Contributor

I presume those casts would stay, it would just be explicit casts between stuff like f64->f32 or f32->f64 wouldn't exist (since they would be implicit if safe, and if not safe I guess either there would be a builtin or some other way).

@andrewrk
Copy link
Member Author

andrewrk commented Jun 7, 2018

@raulgrell we would add @intToFloat and @floatToInt

        pub fn cast(self: &const Self, comptime C: type) Vec2T(C) {
            return switch (@typeInfo(C)) {
                .Int => Vec2T(C) {
                    .x = @floatToInt(C, self.x),
                    .y = @floatToInt(C, self.y),
                },
                .Float => Vec2T(C) {
                    .x = @intToFloat(C, self.x),
                    .y = @intToFloat(C, self.y),
                },
                else => unreachable,
            }
        }

(this example assumes #683)

@andrewrk andrewrk added the accepted This proposal is planned. label Jun 9, 2018
@bheads
Copy link

bheads commented Jun 12, 2018

Why not just @cast(T, val)? only one function to remember and easy to grep for in a program,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

6 participants