teal-language / tl

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

Circular type dependency in multiple files does not work #355

Closed jmcmorris closed 2 years ago

jmcmorris commented 3 years ago

Hello! I recently ran into an issue with a circular type dependency in multiple files. Here is what the simplified setup looks like.

-- bar.tl
local Foo = require('foo')
local record Bar
    foo: Foo
end
return Bar
-- foo.tl
local Bar = require('bar')
local record Foo
    bar: Bar
end
return Foo

My actual use case is a bit more complicated than this but still completely reasonable. I have an Entity that has a field for each component type and one of my components has a function which tries to use an Entity in order to get a component on it.

Is this a feature planned for Teal? Thanks!

lenscas commented 3 years ago

I believe that lua itself doesn't like circular dependencies (https://stackoverflow.com/questions/13964764/lua-how-to-avoid-circular-requires ) so teal can't support it the normal way either.

It might be possible for teal to have a "type only" require, which won't end up in the generated lua and thus might be able to support circular dependencies but that probably opens up another can of worms.

jmcmorris commented 3 years ago

I agree and try to avoid circular dependencies. The only reason this is a problem for me is that I need to have the type information for Teal check to pass. My Entity prototype is in a Entity.d.tl that contains only the prototype (since it is defined in C++). If there was a "type only" require then this would fix this specific issue. Another possible fix, I my understanding of Teal is correct, would be to depth first parse on all record prototype data before doing the type check.

I have very limited experience in this field so I'll leave it up to you experts. Just wanted to see if there were any plans for making this work or if I need to redesign things.

lenscas commented 3 years ago

using require to load an Entity.d.tl file will still leave this require in the lua code, so unless your C++ is a lua module this will probably cause problems. You can use the preload option in the tlconfig.lua to load these type definitions without the need for require https://github.com/teal-language/tl/blob/master/docs/compiler_options.md#list-of-compiler-options

jmcmorris commented 3 years ago

@lenscas This is exactly what I'm doing. However, ultimately this preload is just a require within tl and that is what is causing the circular type dependency.

hishamhm commented 3 years ago

Hi @jmcmorris, thanks for the feedback!

Is this a feature planned for Teal? Thanks!

Currently not, since, as @lenscas said, Lua's require itself does not like circular dependencies, so the usual pattern is to design the dependency chain as to avoid them. But it's important to hear feedback like this to understand the implications for users.

Note that within the same module, circular dependencies in records are not a problem since names are resolved lazily.

If there was a "type only" require then this would fix this specific issue. Another possible fix, I my understanding of Teal is correct, would be to depth first parse on all record prototype data before doing the type check.

I think both proposals would amount to roughly the same thing. The Teal compiler currently has a very simple AST-traversing internal design, and I'd rather not add multi-pass processing for the sake of circular dependencies alone.

Just wanted to see if there were any plans for making this work or if I need to redesign things.

If you're making a preloaded module that exports global types, at this point I'd just make one combined module with the circularly-dependent types.

In general, I think preload in tlconfig.lua should be used for applications and environment which preload globals (such as love in Love2D, ngx in OpenResty, or Lua VMs embedded in applications that expose their own globals), and not as a workaround for limitations of require.

DragonDePlatino commented 3 years ago

I'm designing an Entity Component System for a game in Teal and encountering the same issue here. Here's a more concrete example:

--- @class Parent
--- @field children Child[]
local Parent = {}
Parent.__index = Parent

--- @return Parent
function Parent:new()
    return setmetatable({
        children = {}
    }, self)
end

--- @param child Child
--- @return Child
function Parent:addChild(child)
    child.parent = self
    self.children[#self.children + 1] = child
    return child
end

--- @param payload string
function Parent:notify(payload)
    print('Child notified: ' .. payload)
end

return Parent
--- @class Child
--- @field parent Parent
local Child = {}
Child.__index = Child

--- @return Child
function Child:new()
    return setmetatable({}, self)
end

function Child:notifyParent()
    if not self.parent then return end
    self.parent:notify('Message')
end

function Child:throwError()
    -- LDoc warns about an undefined global here
    local parent = Parent:new()
end

return Child

All of this is valid type-checked Lua that works at runtime, except the Child:throwError() method. The pattern shows up in ECS, UI systems, Observer pattern and lots of other places. At this stage there are relationships you can express in plain Lua + LDoc that aren't possible in Teal. Two possible solutions:

  1. A Teal construct that lets you to do a types-only import with no added require at runtime.
  2. Pre-scanning all detected files and including those global types like vscode-lua does.

I know either solution would violate the single-pass pureness of TL but it feels like a necessary evil to model real-world scenarios like this. LDoc is great but I would love if you could express this in Teal since it's way less verbose!

DragonDePlatino commented 3 years ago

Found a temporary solution for this, maybe it'd hint for at a possible solution? Here's a cut-down example of the final code:

entity.tl

local record Entity
    {ComponentType}
    id: integer
end
(Entity as metatable<Entity>).__index = Entity

function Entity:new(id: integer): Entity
    ...
end

function Entity:add(component: ComponentType)
    ...
end

function Entity:remove(cid: integer)
    ...
end

function Entity:read(m: Message): integer
    ...
    for i = 1, #self do
        local cmp = self[i]
        local result = cmp:read(self, m)
    end
    ...
end

return Entity

component.tl

local record Component
    id: integer
end
(Component as metatable<Component>).__index = Component

function Component:new(): Component
    ...
end

function Component:read(e: EntityType, m: Message): integer
    ...
end

return Component

global.d.ts


global type ComponentType = record
    id: integer
    read: function(self: ComponentType, e: EntityType, m: Message): integer
end

global type EntityType = record
    {ComponentType}
    id: integer
end

Summed up, for every concrete Foo type in the application, the compiler could define a global FooType which is only used internally. FooType is safe to use in places like type annotations or function parameters, but you cannot call concrete methods on that. So a call like cmp:read() is safe to do without requiring Component, but Component:new() is not! Then we'd get behavior closer to the vanilla Lua.

hishamhm commented 2 years ago

In the code in latest master, circular type dependencies across multiple files can be dealt with using global type forward definitions (see the Global Variables section in the Tutorial). This is not super-ideal but it's a solution for now (and the simplest one that avoids a multipass compiler).