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

fix: regression related to type narrow and generic since v3.10.1 #2898

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

tomlau10
Copy link
Contributor

While giving reply in an old issue, I found a regression issue related to @overload and @generic: #723 (comment)
After doing git bisect, I found that it is introduced in this commit of v3.10.1 : 8ecec08

Minimal Example

---@generic T
---@param v T
---@return T
local function f1(v) end
local r1 = f1(1) --> integer, good

---@generic T
---@param v T
---@return T
---@overload fun(): nil
local function f2(v) end
local r2 = f2(1) --> unknown, bad

Expected Result

---@generic T
---@param v T
---@return T
---@overload fun(): nil
local function f2(v) end
local r2 = f2(1) --> integer, good

中文版

我在回覆一個舊 issue 的時候,發現一個關於 @overload@generic 的 regression 問題,具體例子見上邊。
大概是當 function 同時有 overload 和 generic param,並且 overload 的 function signature 比帶 generic 的短 就會觸發 (?)

用 git bisect 找到是這個 commit 開始出現的: 8ecec08
這個 commit 當時是為了 fix #2765 所引起的 runtime error,所以可以肯定是 bugfix commit 引起了不知明 side effect 🤔

估計成因

  • 當時 fix: inconsistent type narrow due to outdated node caches of call.args #2765 是為了讓 call.args 能在 type narrow 後做 recompute,所以直接把 call.args 的 node cache 刪掉了,但卻有可能引起 runtime error
  • 8ecec08 的 fix 則是 直接將所有 call.args node cache 先設成 empty 來解決 runtime error
  • 我懷疑是如果某個 call.args 的 node cache 本身未存在,就直接設成 empty object,反而會引起之後的 infer 錯誤
    => empty infer => 就是 unknown 🤔
  • 所以這裡的 fix 是 只有 vm.getNode() 存在 時,才會 overwrite 成 empty new node

@sumneko sumneko merged commit be9ce62 into LuaLS:master Oct 21, 2024
11 checks passed
@tomlau10 tomlau10 deleted the fix/type_narrow_regression branch October 24, 2024 06:10
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

Successfully merging this pull request may close these issues.

2 participants