teal-language / tl

The compiler for Teal, a typed dialect of Lua
MIT License
2.02k stars 101 forks source link

[next branch] `where` conflicts with `<total>` #749

Open Andre-LA opened 3 weeks ago

Andre-LA commented 3 weeks ago

Code to reproduce:

local interface Op
    op: string
end

local interface Binary
    is Op
    left: number
    right: number
end

local record Add
    is Binary
    -- where self.op == '+' -- removing the comment triggers an error
end

local sum <total>: Add = {
    op = '+',
    left = 10,
    right = 20
}

print(sum.op, sum.left, sum.right)
hishamhm commented 3 weeks ago

This is happening because the presence of any metamethod (even the __is pseudo-metamethod that is implied by where) is currently marking a table as non-total:

https://github.com/teal-language/tl/blob/master/tl.tl#L9015-L9017

The intention was that <total> was only defined for "known-to-be-well-behaved" types (after all you can do all sorts of crazy things with metamethods). Maybe that was just being overzealous from my part. Simply removing that check makes this case pass. I'm now wondering whether I should just remove it, or if I should downgrade it to something to "check for the presence of __index and/or __newindex").

Andre-LA commented 3 weeks ago

To be honest I generally rely on new functions to ensure the initialization of tables are correct, because of this I don't use <total> often for records (I found this one by accident while testing).

Maybe should records also has the foo?: number syntax? like that:

local record Foo
    bar: number
    baz?: number
end

local foo: Foo = {
    bar = 10
    -- compiler accepts no baz initialization because it's "baz?"
}

print(foo.bar, foo.baz)

So the advantage of "init-checking" of <total> is preserved, the optional init is available and can be manually specified case by case, and <total> could be constrained just on the original maps-of-enums use case you cited on my previous report.

The downside I suppose is backward compatibility, since it would start checking keys presence (without <total>) unlike before.

Frityet commented 1 week ago

To be honest I generally rely on new functions to ensure the initialization of tables are correct, because of this I don't use <total> often for records (I found this one by accident while testing).

Maybe should records also has the foo?: number syntax? like that:

local record Foo
  bar: number
  baz?: number
end

local foo: Foo = {
  bar = 10
  -- compiler accepts no baz initialization because it's "baz?"
}

print(foo.bar, foo.baz)

So the advantage of "init-checking" of <total> is preserved, the optional init is available and can be manually specified case by case, and <total> could be constrained just on the original maps-of-enums use case you cited on my previous report.

The downside I suppose is backward compatibility, since it would start checking keys presence (without <total>) unlike before.

optional record members is something that would help so much, in general I wish we had nillable types, but at least optional record members would be nice