[vim/vim] vim9class work/bugs TODO for vim9.1 (Discussion #13041)
errael
(Short term discussion)
Over a week ago, there was a one week estimate to take bug fixes. Some notable missing features were discovered:
- access control was not implemented (could write to privates)
- reading/writing statics outside of a class not implemented
Basic access control was fixed, reading statics outside of a class fixed.
Variety of other fixes, for example garbage collection of class/interface.
I don't think there are any open issues regarding specification of syntax/semantics.
Issues I'm aware of and/or I'm not sure about
- writing statics in s
:defoutside of a class - more access control situations
- script level access issues, maybe only statics
Still not feature complete because of 1.
The second could be considered bugs, the third hasn't been triaged (AFAIK), could be missing feature or bugs.
Putting together tests for 2. over the last day. Some bugs found/fixed; Good/Bad news, it's still pretty easy to find bugs (though I can be brutal, so maybe it's not that bad). I've add some fails below, depending can open them as bugs (certainly will if they are not fixed for 9.1)
@yegappan @chrisbra Those at a higher pay grade than I will have to contemplate, meditate, and otherwise look at the situation, and make a scheduling update. I put TODO in the tile, but things change...
Following run with 9.0.1872 and PR #13032
script level static access example
line 8:
E1326: Member not found on object "A": svar1
vim9script
class A
public static svar1: number
endclass
var oa = A.new()
echo oa.svar1
weird name duplication error
line 5:
E1369: Duplicate member: s_var
Flip the order of the declarations, and there is no error. Probably off by 1 for duplicate name check
vim9script
class C
public static s_var2: number
public static s_var: number
endclass
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
(Short term discussion)
Over a week ago, there was a one week estimate to take bug fixes. Some notable missing features were discovered:
- access control was not implemented (could write to privates)
- reading/writing statics outside of a class not implemented
Basic access control was fixed, reading statics outside of a class fixed.
Variety of other fixes, for example garbage collection of class/interface.
I don't think there are any open issues regarding specification of syntax/semantics.Issues I'm aware of and/or I'm not sure about
- writing statics in s
:defoutside of a class- more access control situations
- script level access issues, maybe only statics
Still not feature complete because of 1.
The second could be considered bugs, the third hasn't been triaged (AFAIK), could be missing feature or bugs.Putting together tests for 2. over the last day. Some bugs found/fixed; Good/Bad news, it's still pretty easy to find bugs (though I can be brutal, so maybe it's not that bad). I've add some fails below, depending can open them as bugs (certainly will if they are not fixed for 9.1)
@yegappan @chrisbra Those at a higher pay grade than I will have to contemplate, meditate, and otherwise look at the situation, and make a scheduling update. I put TODO in the tile, but things change...
Following run with 9.0.1872 and PR #13032
script level static access example
line 8: E1326: Member not found on object "A": svar1vim9script class A public static svar1: number endclass var oa = A.new() echo oa.svar1weird name duplication error
vim-dev ML
On Tue, Sep 5, 2023 at 8:20 PM errael ***@***.***> wrote:
> (Short term discussion)
>
> Over a week ago, there was a one week estimate to take bug fixes. Some
> notable missing features were discovered:
>
> 2. reading/writing statics outside of a class not implemented
>
> Basic access control was fixed, *reading* statics outside of a class
> Variety of other fixes, for example garbage collection of class/interface.
> I don't think there are any open issues regarding specification of
> syntax/semantics.
>
> Issues I'm aware of and/or I'm not sure about
>
> 2. more access control situations
> 3. script level access issues, maybe only statics
>
Thanks for summarizing the outstanding issues.
>
>
> The second could be considered bugs, the third hasn't been triaged
> (AFAIK), could be missing feature or bugs.
>
> Putting together tests for 2. over the last day. Some bugs found/fixed;
> Good/Bad news, it's still pretty easy to find bugs (though I can be brutal,
> so maybe it's not that bad). I've add some fails below, depending can open
> them as bugs (certainly will if they are not fixed for 9.1)
>
> <https://github.com/chrisbra> Those at a higher pay grade than I will
> make a scheduling update. I put TODO in the tile, but things change...
>
> Following run with 9.0.1872 and PR #13032
>
> *script level static access example*
> line 8:
> E1326: Member not found on object "A": svar1
>
> vim9script
>
> class A
> public static svar1: number
> endclass
>
> var oa = A.new()
> echo oa.svar1
>
I have opened PR https://github.com/vim/vim/pull/13042 with a fix for this
issue.
Regards,
Yegappan
> E1369: Duplicate member: s_var
>
> Flip the order of the declarations, and there is no error. Probably off by
> 1 for duplicate name check
>
> vim9script
>
> class C
> public static s_var2: number
> public static s_var: number
> endclass
>
>
>
—
Reply to this email directly, view it on GitHub,.
You are receiving this because you are subscribed to this thread.
Yegappan Lakshmanan
vim-dev ML
>
> Hi,
>
>>
>> (Short term discussion)
>>
>> Over a week ago, there was a one week estimate to take bug fixes. Some notable missing features were discovered:
>>
>> access control was not implemented (could write to privates)
>> reading/writing statics outside of a class not implemented
>>
>> Basic access control was fixed, reading statics outside of a class fixed.
>> Variety of other fixes, for example garbage collection of class/interface.
>> I don't think there are any open issues regarding specification of syntax/semantics.
>>
>> Issues I'm aware of and/or I'm not sure about
>>
>> writing statics in s :def outside of a class
>> more access control situations
>> script level access issues, maybe only statics
>
>
>
>>
>> The second could be considered bugs, the third hasn't been triaged (AFAIK), could be missing feature or bugs.
>>
>> Putting together tests for 2. over the last day. Some bugs found/fixed; Good/Bad news, it's still pretty easy to find bugs (though I can be brutal, so maybe it's not that bad). I've add some fails below, depending can open them as bugs (certainly will if they are not fixed for 9.1)
>>
>> @yegappan @chrisbra Those at a higher pay grade than I will have to contemplate, meditate, and otherwise look at the situation, and make a scheduling update. I put TODO in the tile, but things change...
>>
>> Following run with 9.0.1872 and PR #13032
>>
>> script level static access example
>>
>> line 8:
>> E1326: Member not found on object "A": svar1
>>
>> vim9script
>>
>> class A
>> public static svar1: number
>> endclass
>>
>> var oa = A.new()
>> echo oa.svar1
>>
>> weird name duplication error
>
>
>
Both of these issues are now addressed in the PR.
Regards,
Yegappan
>
>>
>> E1369: Duplicate member: s_var
>>
>> Flip the order of the declarations, and there is no error. Probably off by 1 for duplicate name check
>>
>> vim9script
>>
>> class C
>> public static s_var2: number
>> public static s_var: number
>> endclass
>>
>>
—
Reply to this email directly, view it on GitHub,.
You are receiving this because you are subscribed to this thread.
errael
I don't think there are any open issues regarding specification of syntax/semantics.
Oops, I was wrong. See #13004 . Vim's inheritance model of statics is unlike any known language and offers no performance advantage.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
I see
- re-work: read-static outside of a class
- implement: write-static in
:defoutside of a class - static inheritance issue
- Added another script level failing case to the original message. It has a similar, but different, error message.
The re-work/rewrite is not strictly need, since it is an implementation issue and not user visible, except through disassemble, but doing it the "right way" now, while it's fresh in mind is probably a good idea; it can be deferred until after the second one. The second is primarily a parsing issue I think, assuming load/store classmember can be used. The third needs some deep thought.
@yegappan
I was thinking of doing the first right away, because it is a proof of concept and get me into the groove for the second. Just using the load/store classmember without removing the unneeded code should be quick. There might be an issue with where/how the generate_XXX is called because other instructions may already have been generated.
Following is a detailed look at the first and second items. (TLDR (but I can refer to it); but if you notice a conceptual flaw, LMK)
reading statics outside of a class fixed.
There might be a better fix.
While wondering about #13004 and looking at code I got a better understanding of the vim9script Instruction Set Architecture and how the instructions are encoded/decoded. In addition, getting a better appreciation of the isn_arg, between compilation and runtime. This is particularly relevant because the following is an outstanding issue related to the reading.
writing statics in s :def outside of a class
For reading, even though I looked, I couldn't find an instruction that handled statics (I even thought about adding some). But I think the following is exactly what I was looking for
return sval1
0 LOAD CLASSMEMBER C.sval1
1 RETURN
and
sval1 = 11
0 PUSHNR 11
1 STORE CLASSMEMBER C.sval1
My fundamental error was that I assumed that the object was needed, as well as the index, as with object members. For example
0 LOAD arg[-1] <<< the object
1 OBJ_MEMBER 0 <<< the index encoded in the instruction
But that's not true. And the ITF_MEMBER instructions just added to my confusion. If you look at the disassemble above, the LOAD/STORE CLASSMEMBER have the Class.member encoded in it. The object isn't needed and the index isn't needed; they are implied by Class.name. I believe the read-static task doesn't need an instruction change. Likewise the write static already has the needed instructions. (I might also have had some weird idea the load/store classmember instruction could only be used inside a class)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
dkearns
script level static access example (fixed #13042)
line 8: E1326: Member not found on object "A": svar1vim9script class A public static svar1: number endclass var oa = A.new() echo oa.svar1
This doesn't look like a bug to me, though the error message could be clearer. Accessing static members through object references has been considered a bad idea for a long time. E.g., the C# compiler will report something like
error CS0176: Member 'A.svar1' cannot be accessed with an instance reference; qualify it with a type name instead
and most linters/style guides will flag it.
More importantly, it's inconsistent with the other design decisions made for Vim9 that are intended to reduce call-site bafflement. There's already two ways to access the static member (bare and type-qualified) a third seems unnecessary when one is considered enough in other contexts.
Refs:
- https://learn.microsoft.com/en-us/dotnet/csharp/misc/cs0176
- https://google.github.io/styleguide/javaguide.html#s6.3-static-members
- https://www.oracle.com/java/technologies/javase/codeconventions-programmingpractices.html#587
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
This doesn't look like a bug to me, though the error message could be clearer
Accessing static members through object references has been considered a bad idea for a long time
I tend to agree; documenting this as an error and improving error messages for 9.1 seems like a good idea. It can always be added in the future if an important use case showed up.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
@yegappan @dkearns @chrisbra Updated intitial post of this discussion. Two open spec syntax/semantic issues.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
I recently made a change to support calling a class method using an object from outside the class context. If we are going to block accessing class members using an object, then we should also block accessing class methods using an object.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
dkearns
Yes, there's no distinction here.
The language used to discuss these concepts in the documentation is a bit unusual to me. "member" is, in my experience, normally a collective term that includes methods. I've had whole exchanges not realising we were only discussing "member variables". :)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
After patch 9.0.1887, class variables and methods can be accessed only using the class name outside a class. These cannot be accessed using an object. Inside a class, these can be referenced either with or without the class name.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Updated initial post, only one thing left unresolved in the spec syntax/semantics.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Currently, accessing a static on an instance var gives obscure error messages.
There C#'s message
error CS0176: Member 'A.svar1' cannot be accessed with an instance reference; qualify it with a type name instead
If there no one working on it, I'll put together a PR that uses a more targeted message. The specific message can be tuner after the PR is opened, but some questions to consider:
The vim9class doc uses "object" and not "instance"; the word instance does not appear at all. "object reference" doesn't appear in vim9class.txt, there's talk of "access an object member".
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
Please go ahead and open a PR for this. I am not working on this. I am working on refactoring the class related code.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
We can standardize on using "object" in the documentation?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Christian Brabandt
and is merged, thanks both!
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Christian Brabandt
yes, I think we can use "object" consistently in the documentation. Thanks for working on that
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Here's two thing that I think are bugs, looking for confirmation. If they do not have source compatibility issues and/or they are not fixed, then when 9.1 is release I can open issues. Has to do with interface hierarchies.
This one looks like it might have a source compatibility issue.
vim9script
# This has two potential issues
interface I1
public this.val1: number
endinterface
# Java uses extends here; extends seems to make more sense.
# Might be indicative of the compatibility issue
interface I2 implements I1
public this.val1: number # <<<<< This feels inconsistent
# when a class extends a class it doesn't
# doesn't have to declare things from
# classes that it extends.
# MIGHT BE A SOURCE COMPATIBILITY ISSUE
public this.val2: number
endinterface
class A implements I2
public this.val1 = 1
public this.val2 = 2
endclass
def FI1(i: I1)
echo "FI1" i.val1
enddef
def FI2(i: I2)
echo "FI2" i.val2
enddef
var a = A.new()
FI2(a) # <<<<< this works
FI1(a) # <<<<< E1013: Argument 1: type mismatch, expected object<I1> but got object<A>
This one has a catch-22
vim9script
interface I1
public this.val1: number
endinterface
interface I2 implements I1
public this.val1: number
public this.val2: number
endinterface
class A implements I2
public this.val1 = 1
public this.val2 = 2
endclass
# This two line class, with an empty body, gets this error
# E1348: Member "val1" of interface "I1" not implemented
#class B extends A implements I1
#endclass
# E1369: Duplicate member: val1
class B extends A implements I1
public this.val1 = 1
endclass
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
I deleted comment about interface inheritance and opened #13085 instead.
Apologies for any inconvenience.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Updated initial post, there's a consensus around the spec syntax/semantics for statics/inheritance. That leaves only one open item #13069. But there a new discussion #13085 which may reveal 9.1 issues.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Devin Weaver
I was watching a YoutTube about OOP from a SmallTalk perspective and something they mentioned was that the difference from most OO languages and the OO from SmallTalk and Ruby is their stance that all Objects can be sent any message and it is up to the receiving object to choose how to manage that or accept it.
The example they mentioned was instead of say an interface does not handle that method call instead it defaults to message unhandled with a way for objects to either declare which messages they respond to or have a catch all missingMethod()
I think the idea was in the shifting from interface mechanics to message passing. Curious of thoughts about this distinction especially with it applicability to Vim9Script.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
(I can open bug reports if that is more convenient, will if these aren't resolve in 9.1 release)
This may be a bug; fixing it could cause a source incompatibility. The doc says
Object methods of the base class can be overruled. The signature (arguments,
argument types and return type) must be exactly the same.
While it doesn't explicitly say access must be the same...
I actually like being able to promote a private method to public in the child class; but it still looks like a bug. Going the other way is not an error either, and in fact both _F() and F() can be invoked; certainly a bug I think.
vim9script
class A
def _F()
echo 'in A._F'
enddef
endclass
class B extends A
# shouldn't this be an error <<<<<<<<<<<<<<<<<<
def F()
echo 'in B.F'
super._F()
enddef
endclass
B.new().F()
vim9script
class A
def F()
echo 'in A.F'
enddef
endclass
class B extends A
# shouldn't this be an error <<<<<<<<<<<<<<<<<<
def _F()
echo 'in B._F'
super.F()
enddef
def G()
echo 'in B.G'
this._F()
enddef
endclass
echo '=== B.new().F()'
# F() is accessible with object<B>
B.new().F()
echo '=== B.new().G()'
B.new().G()
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Duplicate method name for both object/class method not flagged as an error.
This output should be a dup name error, but it runs OK
A.ro_svar 1
in A.WA()
A.ro_svar 3
in static A.WA()
A.ro_svar 5
from this script
vim9script
class A
static ro_svar = 1
public static svar = 3
def WA(arg: number) # <<<
echo 'in A.WA()'
ro_svar = arg
enddef
static def WA(arg: number) # <<<
echo 'in static A.WA()'
ro_svar = arg
enddef
endclass
var oa: A = A.new()
echo 'A.ro_svar' A.ro_svar
oa.WA(3)
echo 'A.ro_svar' A.svar
A.WA(5)
echo 'A.ro_svar' A.ro_svar
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Seems it will take quite an effort to find more bugs in this PR.
(exactly when my build with latest PR finished this morning, a new commit with most tests showed up, got to be a good sign)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
After patch 9.0.1906, 92d9ee5, there are no known major issues (that I know of anyway) for the vim9.1script spec. I'm assuming the spec is now frozen for vim9.1 (which may also not be true).
Maybe there will be a little breathing space before more discussions with proposals are opened to extend vim9class syntax/semantics. No, there won't be; there is already discussion about things mentioned in the vim9script spec which are for the future. Maybe the discussion won't feel quite so urgent.
And there are bugs, which may mean that vim9script that compiles and runs under vim9.1 will not compile if some random bug is fixed. At least I hope that's the case, I'd like it to be OK to fix bugs.
I won't close this until vim9.1 is released.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
dkearns
After patch 9.0.1906, 92d9ee5, there are no known major issues (that I know of anyway) for the vim9.1script spec. I'm assuming the spec is now frozen for vim9.1 (which may also not be true).
#12965 and related typing issues need to be fixed if backward compatibility is required.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
dkearns
Vim9 syntax highlighting needs quite a bit of work. I'll make an issue for this once I've investigated further.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
dkearns
This is probably an implicit task but as I've started playing with these features the help tags need some improvement. E.g., the last three I tried didn't turn up anything private, constructor, new()
I'll add a PR when I have more.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Vim9 syntax highlighting needs quite a bit of work
@dkearns Have you taken a look at https://github.com/lacygoill/vim9-syntax. I've been using it for over a year. Since I've never used legacy vimscript, I can't really compare with the default highlighing. Some favorite things with @lacygoill 's vim9-syntax is the mutli-line lambda highlighting, and type highlighting. The configuration options might give you some clues about it.
@chrisbra @yegappan Perhaps including this with the release is something to consider.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
dkearns
@dkearns Have you taken a look at https://github.com/lacygoill/vim9-syntax.
Thanks, I've been using that for some time too but I'm running with the default highlighting at the moment to see what needs implementing. I'll fix the issues I'm aware of soon and offer a PR.
Since I've never used legacy vimscript
Hmm, this is a surprising disclosure. Where have you come from? :)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
vim9-syntax recently added packadd! vim9-syntax capability, so it's easy to flip back and forth.
Where have you come from?
jVi ;-) I started more participation in open source source project several years ago, and finally wanted to get a better feeling for, and improve, my merge skills. I came across Splice and not too long after vim9script was becoming available. Since I've always felt guilty about not learning legacy vimscript (but my sanity is too important) porting Splice (vim/python hybrid) to a pure vim9script solution....; two birds with one stone. With vim9class becoming usable, I've been using pure vim Splice9 for a couple months.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
dkearns
Wow, jVi takes me back...almost twenty five years. I'm impressed it's still being maintained.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
dkearns
This can be a bit difficult to support in a statically typed language but it is possible. Here's a couple of approaches that spring to mind:
- https://crystal-lang.org/reference/1.9/syntax_and_semantics/macros/hooks.html
- https://learn.microsoft.com/en-us/dotnet/csharp/advanced-topics/interop/walkthrough-creating-and-using-dynamic-objects
If you want to continue this or raise any other ideas you have could you please create a dedicated discussion thread? Thanks.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Wow, jVi ... still being maintained
I'm amazed myself; considering my maintenance history, it's a testament to how important vim is to me. Add a feature once in a great while and it doesn't quite feel like maintenance.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Update intro post. Feels so close...
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yee Cheng Chin
This is also how Objective-C works. But it's telling that when Apple moved to Swift, it no longer uses message passing. I think it has nice properties but it's generally not caught on. I don't think vim9script should really reinvent the wheel on such large language features, IMO. But yes I agree this would deserve a separate thread.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yee Cheng Chin
What about :enum and :type? Below are the implementation and documentation, which doesn't inspire confidence:
https://github.com/vim/vim/blob/e379e21ddc2dc12abea9dcfd0708e27788bb568c/src/vim9class.c#L2083-L2099
I actually didn't even know :enum existed and was only looking into into them when thinking about command-line completion for expressions (e.g. :let g:some_var=<Tab>).
I haven't been paying too much attention to vim9script and vim9.1 release so not sure if these have been discussed. If they aren't done by vim9.1, maybe the documentation should be cleaned up for now to avoid giving the wrong impression that the feature works?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
- Reserve dunder,
__, prefix in any vim9class related name for future features. Give error if used.
I have now added the support for dunder methods in #13238.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
Yegappan Lakshmanan
The PR #13238 is the first step in supporting user-defined types using the ":type" command.
Yes. We need to update the documentation to state that ":enum" and ":type" are not yet implemented.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.
errael
Final update (Plus ça change, plus c’est la même chose)
There's a few things pending, but nothing new for a while.
(bugs will never go away)
—
Reply to this email directly, view it on GitHub,.
You are receiving this because you commented.
errael
Closed #13041 as resolved.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you commented.