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

Proposal: Require explicit cast between different type aliases even if they are compatible #18991

Closed
serramatutu opened this issue Feb 19, 2024 · 3 comments

Comments

@serramatutu
Copy link

serramatutu commented Feb 19, 2024

Currently, the Zig compiler does not fail when type aliases are used interchangeably, and I argue it should. I will motivate my proposal with a concrete example.

In the code below, although OwnerId and PetId are technically the same thing (both are u64), they semantically represent different things (an owner and a pet) which are not interchangeable. Functions that accept OwnerId should not be called with PetId and vice-versa, even though the underlying parameter types are perfectly compatible.

Currently, the Zig compiler sees no issue with this, and will compile without errors or warnings.

// You can copy this code as-is and compile it with Zig 0.11.0 with no issues.

const std = @import("std");

const OwnerId = u64;
const PetId = u64;

pub fn getMostPopularPet() PetId {
    // Get the most famous pet somehow
    return 0;
}

pub fn getPetIdByOwnerId(owner_id: OwnerId) PetId {
    // Fetch the PetId by OwnerId somehow
    std.debug.print("owner_id: {?}\n", .{owner_id});
    return 0;
}

pub fn main() !void {
    const famous_pet: PetId = getMostPopularPet();

    // getPetIdByOwnerId is called with a PetId, even though it expects an OwnerId.
    // This is an application logic error that could be enforced by robust type aliases.
    const pet_id = getPetIdByOwnerId(famous_pet);

    std.debug.print("pet_id: {?}\n", .{pet_id});
}

My proposal is to make the implicit cast of famous_pet to OwnerId raise a compilation error like so

'famous_pet' is a PetId but 'getPetIdByOwnerId' expects a OwnerId. If this is really your intention, explicitly cast it with @as(OwnerId, famous_pet).

In case the user really knows what they are doing and are OK with passing a PetId into an OwnerId, they should tell the compiler of their intentions with a @as(OwnerId, famous_pet). This cast would result in a no-op and the code would work as normal.

I believe this addition would be especially useful for large applications with lots of different entities (users, accounts, repositories, teams, you name it) which relate to each other and make these kinds of mistakes more likely.

@serramatutu serramatutu changed the title Proposal: Require explicit cast between type aliases even if they are compatible Proposal: Require explicit cast between different type aliases even if they are compatible Feb 19, 2024
@schmee
Copy link
Contributor

schmee commented Feb 19, 2024

Related (duplicate?): #1595

@mlugg
Copy link
Member

mlugg commented Feb 19, 2024

Type aliases in Zig are far more frequently used as just that: aliases. You'll see things like const ArrayList = std.ArrayList, const Build = std.Build, etc dotted all throughout the compiler, standard library, and third-party code. This pattern is encouraged; it makes code more readable.

What you really seem to want is some method to create distinct types, as in the proposal linked above (#1595). When you're working with integers, there is a convention for this today: using a non-exhuastive enum.

const MyId = enum(u64) {
    // This marks the enum as "nonexhuastive", so that every `u64` can be used as an
    // enum variant, even if we don't have an explicit name for it.
    _,
};

This solves the problem for the most common class of type (note that enums are guaranteed to have the same ABI as their backing integer), with the added advantage that it can have declarations (including functions) and explicit tags for special values (e.g. I could define none = std.math.maxInt(u64) to have a convenient-to-use "null"-esque value). If you have a use case not satisfied by this, I'd suggest posting it in #1595 instead.

@mlugg mlugg closed this as not planned Won't fix, can't repro, duplicate, stale Feb 19, 2024
@serramatutu
Copy link
Author

@mlugg #1595 Seems to be discussing exactly what I meant with this. Sorry for creating a duplicate, I couldn't find that issue on my own :)

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

3 participants