[vim/vim] Vim9: ":h type-checking" can be improved (#7881)
lacygoill
Here's what the help says at the end of :h type-checking:
One consequence is that the item type of a list or dict given to map() must
not change. This will give an error in compiled code: >
map([1, 2, 3], (i, v) => 'item ' .. i)
E1012: Type mismatch; expected list but got list
Instead use |mapnew()|.
There are a few issues with this paragraph.
First, the error is not:
E1012: Type mismatch; expected list<number> but got list<string>
It's:
E1012: Type mismatch; expected number but got string
Second, there's no good reason to mention "in compiled code".
One reason could be that the error is only raised in a :def function. That's not the case; it's also raised at the script level:
vim9script map([1, 2, 3], (i, v) => 'item ' .. i)
E1012: Type mismatch; expected number but got string
One could argue that even though the code is written at the script level, the lambda is still compiled. That's true, but only since 8.2.2527. This paragraph was written well before this patch, so I doubt this explains "in compiled code".
Another reason could be that – when the code is compiled – the error is raised at compile time; not at runtime. That's not the case either:
vim9script def Func() map([1, 2, 3], (i, v) => 'item ' .. i) enddef defcompile
no error
I think "in compiled code" should be replaced with "at runtime".
Third, the help says that the item type must not change:
One consequence is that the item type of a list or dict given to map() must
not change.
That's not entirely true. It can change if the old last subtype is any, and the new one is more specific (number, string, ...):
vim9script def Func() var l: list<dict<any>> = [{a: 0, b: ''}] map(l, (_, v: dict<any>): dict<number> => ({c: 0})) enddef Func()
no error
Notice that no error was raised, even though the old type of the list items was dict<any>, while the new one is dict<number>. It changed without Vim complaining. I think it's working as intended (right?), because the new type is compatible with the old one. It might be similar to this:
vim9script var x: list<any> = [1, 2, 3]
This snippet doesn't raise any error, even though we've declared that x was a list of mixed values, while in reality we assign it a list of numbers.
Anyway, if this is working as intended, I think the help should mention this:
One consequence is that the item type of a list or dict given to map() must
not change; unless the last subtype is <any>, in which case, it is allowed
to replace it with a more specific type.
Fourth, this rule is not enforced only for map(); it also affects extend() and flatten(). Maybe the help should mention them too.
As a suggestion here is patch:
diff --git a/runtime/doc/vim9.txt b/runtime/doc/vim9.txt index bde3c9b7b..361a55e9a 100644 --- a/runtime/doc/vim9.txt +++ b/runtime/doc/vim9.txt @@ -307,14 +307,14 @@ Example: > const myList = [1, 2] myList = [3, 4] # Error! myList[0] = 9 # Error! - muList->add(3) # Error! + myList->add(3) # Error! < *:final* `:final` is used for making only the variable a constant, the value can be changed. This is well known from Java. Example: > final myList = [1, 2] myList = [3, 4] # Error! myList[0] = 9 # OK - muList->add(3) # OK + myList->add(3) # OK It is common to write constants as ALL_CAPS, but you don't have to. @@ -1028,11 +1028,13 @@ an error, thus breaking backwards compatibility. For example: - Using a string value when setting a number options. - Using a number where a string is expected. *E1024* -One consequence is that the item type of a list or dict given to map() must -not change. This will give an error in compiled code: > +One consequence is that the item type of a list or dict given to map(), +extend(), or flatten() must not change; unless the last subtype is <any>, in +which case, it is allowed to replace it with a more specific type. This will +give an error at runtime: > map([1, 2, 3], (i, v) => 'item ' .. i) - E1012: Type mismatch; expected list<number> but got list<string> -Instead use |mapnew()|. + E1012: Type mismatch; expected number but got string +Instead use |mapnew()|, |extendnew()|, or |flattennew()|. ==============================================================================
Note that the patch also fixes 2 unrelated typos which are currently present on line 310:
muList->add(3) # Error!
and 317:
muList->add(3) # OK
In both cases, muList should be replaced with myList.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.
lacygoill
Maybe the comma after "in which case" is superfluous:
diff --git a/runtime/doc/vim9.txt b/runtime/doc/vim9.txt
index bde3c9b7b..ee7b43e38 100644
--- a/runtime/doc/vim9.txt +++ b/runtime/doc/vim9.txt @@ -307,14 +307,14 @@ Example: > const myList = [1, 2] myList = [3, 4] # Error! myList[0] = 9 # Error! - muList->add(3) # Error! + myList->add(3) # Error! < *:final* `:final` is used for making only the variable a constant, the value can be changed. This is well known from Java. Example: > final myList = [1, 2] myList = [3, 4] # Error! myList[0] = 9 # OK - muList->add(3) # OK + myList->add(3) # OK It is common to write constants as ALL_CAPS, but you don't have to. @@ -1028,11 +1028,13 @@ an error, thus breaking backwards compatibility. For example: - Using a string value when setting a number options. - Using a number where a string is expected. *E1024* -One consequence is that the item type of a list or dict given to map() must -not change. This will give an error in compiled code: > +One consequence is that the item type of a list or dict given to map(), +extend(), or flatten() must not change; unless the last subtype is <any>, in
+which case it is allowed to replace it with a more specific type. This will+give an error at runtime: > map([1, 2, 3], (i, v) => 'item ' .. i) - E1012: Type mismatch; expected list<number> but got list<string> -Instead use |mapnew()|. + E1012: Type mismatch; expected number but got string +Instead use |mapnew()|, |extendnew()|, or |flattennew()|. ==============================================================================
—
lacygoill
I think "in compiled code" should be replaced with "at runtime".
This is especially important to explain why this kind of issue can't be avoided. map(), extend(), flatten() don't care about the type of a declared variable received as first argument, at compile time. They only care about the type of the value they get at runtime.
lacygoill
They only care about the type of the value they get at runtime.
To be more accurate, they only care about the type they can infer from the value.
lacygoill
If we use the word "subtype" for the old value, we should use it for the new value too (to be consistent). New patch:
diff --git a/runtime/doc/vim9.txt b/runtime/doc/vim9.txt
index bde3c9b7b..ce05d5567 100644
--- a/runtime/doc/vim9.txt +++ b/runtime/doc/vim9.txt @@ -307,14 +307,14 @@ Example: > const myList = [1, 2] myList = [3, 4] # Error! myList[0] = 9 # Error! - muList->add(3) # Error! + myList->add(3) # Error! < *:final* `:final` is used for making only the variable a constant, the value can be changed. This is well known from Java. Example: > final myList = [1, 2] myList = [3, 4] # Error! myList[0] = 9 # OK - muList->add(3) # OK + myList->add(3) # OK It is common to write constants as ALL_CAPS, but you don't have to. @@ -1028,11 +1028,13 @@ an error, thus breaking backwards compatibility. For example: - Using a string value when setting a number options. - Using a number where a string is expected. *E1024* -One consequence is that the item type of a list or dict given to map() must -not change. This will give an error in compiled code: > +One consequence is that the item type of a list or dict given to map(), +extend(), or flatten() must not change; unless the last subtype is <any>, in
+which case it is allowed to replace it with a more specific subtype. This will+give an error at runtime: > map([1, 2, 3], (i, v) => 'item ' .. i) - E1012: Type mismatch; expected list<number> but got list<string> -Instead use |mapnew()|. + E1012: Type mismatch; expected number but got string +Instead use |mapnew()|, |extendnew()|, or |flattennew()|. ==============================================================================
—
Bram Moolenaar
Thanks, I'll include it and make it a bit more readable.
Bram Moolenaar
Closed #7881.