Skip to content

TypeInfo.Pointer sentinel value is not always optional #3816

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

Closed
marler8997 opened this issue Dec 1, 2019 · 6 comments
Closed

TypeInfo.Pointer sentinel value is not always optional #3816

marler8997 opened this issue Dec 1, 2019 · 6 comments

Comments

@marler8997
Copy link
Contributor

marler8997 commented Dec 1, 2019

In this example program, the three calls to ptrInfo should all have the same sentinel value type, but each one produces a different type:

const std = @import("std");
const TypeInfo = @import("builtin").TypeInfo;

fn ptrInfo(sentinel: var) TypeInfo.Pointer {
    return TypeInfo.Pointer {
        .is_const = false,
        .is_volatile = false,
        .alignment = 0,
        .child = u8,
        .is_allowzero = false,
        .size = .Many,
        .sentinel = sentinel,
    };
}

pub fn main() void {
    std.debug.warn("{}\n", @typeName(@typeOf(ptrInfo(0).sentinel)));
    std.debug.warn("{}\n", @typeName(@typeOf(ptrInfo(@as(u8, 0)).sentinel)));
    std.debug.warn("{}\n", @typeName(@typeOf(ptrInfo(@as(?u8, 0)).sentinel)));
}
$ zig run bug.zig
comptime_int
u8
?u8

This causes a problem in ir.cpp because it always expects this to be an optional type.

@andrewrk
Copy link
Member

andrewrk commented Dec 6, 2019

This is actually working as designed, because the sentinel field is var:

        sentinel: var,

If you change the test to:

const std = @import("std");
const TypeInfo = @import("builtin").TypeInfo;

fn ptrInfo(sentinel: var) TypeInfo.Pointer {
    return @typeInfo(@Type(TypeInfo{
        .Pointer = .{
            .is_const = false,
            .is_volatile = false,
            .alignment = 0,
            .child = u8,
            .is_allowzero = false,
            .size = .Many,
            .sentinel = sentinel,
        },
    })).Pointer;
}

pub fn main() void {
    std.debug.warn("{}\n", @typeName(@typeOf(ptrInfo(0).sentinel)));
    std.debug.warn("{}\n", @typeName(@typeOf(ptrInfo(@as(u8, 0)).sentinel)));
    std.debug.warn("{}\n", @typeName(@typeOf(ptrInfo(@as(?u8, 0)).sentinel)));
}

Then the output is:

$ ./test
?u8
?u8
?u8

@andrewrk andrewrk closed this as completed Dec 6, 2019
@marler8997
Copy link
Contributor Author

This is actually working as designed, because the sentinel field is var:

Ok. I think this might warrant some special notice in the zig documentation. Something like:

The sentinel field in TypeInfo.Pointer is of type var. The compiler will accept any type that is implicitly convertible to the pointer's child type. However, @typeOf(sentinel) does not necessarily match the child type. Instead, @typeOf(sentinel) is determined by the return type of the sentinel value expression. For example, @typeOf(sentinel) for pointer type [*:0]u8 would be comptime_int rather than u8.

@marler8997
Copy link
Contributor Author

I've created a PR to clarify the behavior here: #3861

@marler8997
Copy link
Contributor Author

marler8997 commented Dec 8, 2019

Another thing I realized is that with this behavior, we are breaking what I thought was going to be an invariant. Namely that for any TypeInfo, say ti, the following would be true:

fn typeInfoInvariant(comptime ti: TypeInfo) void {
    std.debug.assert(ti == @typeInfo(@Type(ti)));
}

Since the sentinel value can be converted from one type to another once it is passed to @Type, we've broken this invariant property. Is this what we want?

If we're not sure, maybe we should repoen this issue?

@andrewrk
Copy link
Member

andrewrk commented Dec 8, 2019

The important invariant is @Type(@typeInfo(T)) == T. Is there a problem that the other lack of invariant causes?

@marler8997
Copy link
Contributor Author

The important invariant is @type(@typeinfo(T)) == T. Is there a problem that the other lack of invariant causes?

True. Maybe that is good enough.

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

2 participants