teal-language / tl

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

0.15.1: type narrowing regression #617

Closed lewis6991 closed 1 year ago

lewis6991 commented 1 year ago

I have the function:

local function handle_writer(pipe: uv.Pipe, x: {string} | string)
  if x is {string} then
    for i, v in ipairs(x) do
      pipe:write(v)
      if i ~= #x then
        pipe:write("\n")
      else
        pipe:write("\n", function()
          try_close(pipe)
        end)
      end
    end
  elseif x then
    -- write is string
    pipe:write(x, function()
      try_close(pipe)
    end)
  end
end

Gives error:

cannot use operator '#' on type {string} | string
hishamhm commented 1 year ago

This one is going to be tricky to solve.

The current implementation of flow-typing for type narrowing is very simplistic, it does not do dataflow analysis or calculate fixpoints. So, to stay on the safe side, the presence of a loop makes it forget the previous narrowing facts (because later code in the loop could invalidate the fact). In 0.14 this was already the case for while and repeat loops; 0.15 added this restriction for for loops for consistency, and that is showing in your example. (In other words, if you nested that for loop with a trivial repeat until false, that #x would fail in 0.14 as well.)

The simplest approach I can think of to make the narrowing smarter without having to implement a proper dependency analysis (which I'll eventually have to, but not right now) would be to, instead of always dropping the narrowing on loop entry, making it check first if the variable is assigned anywhere in the loop, and only drop if it is assigned. At least that would be a linear lookahead. (Of course, that doesn't account for indirect assignments via closures, but then again the current code doesn't either.)

hishamhm commented 1 year ago

The simplest approach I can think of to make the narrowing smarter without having to implement a proper dependency analysis (which I'll eventually have to, but not right now) would be to, instead of always dropping the narrowing on loop entry, making it check first if the variable is assigned anywhere in the loop, and only drop if it is assigned. At least that would be a linear lookahead. (Of course, that doesn't account for indirect assignments via closures, but then again the current code doesn't either.)

I implemented this in #622!