[vim/vim] Class members are accesible only from the class where they are defined. (PR #13086)

Yegappan Lakshmanan

unread,

Sep 13, 2023, 10:50:05 PM9/13/23

to vim/vim, Subscribed

Based on the #13004 discussion, the following changes are made:

Static variables and methods are accessible only using the class name and inside the class where they are defined.
Static variables and methods can be used without the class name in the class where they are defined.
Static variables of a super class are not copied to the sub class.
A sub class can declare a class variable with the same name as the super class.
Use more clear error messages.

This aligns the Vim9 class variable/method implementation with the Dart implementation.

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/13086

Commit Summary

afde8c2 Class members are accesible only from the class where they are defined.

File Changes

(9 files)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

codecov[bot]

unread,

Sep 13, 2023, 11:00:51 PM9/13/23

to vim/vim, Subscribed

Codecov Report

Merging #13086 (afde8c2) into master (6ffcc58) will increase coverage by 1.26%.
The diff coverage is 87.96%.

@@            Coverage Diff             @@
##           master   #13086      +/-   ##
==========================================
+ Coverage   81.46%   82.73%   +1.26%     
==========================================
  Files         160      150      -10     
  Lines      194970   181902   -13068     
  Branches    43747    40881    -2866     
==========================================
- Hits       158841   150500    -8341     
+ Misses      23393    18450    -4943     
- Partials    12736    12952     +216

Flag	Coverage Δ
huge-clang-Array	`82.73% <87.96%> (+<0.01%)`	⬆️
linux	`82.73% <87.96%> (+<0.01%)`	⬆️
mingw-x64-HUGE	`?`
windows	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/vim9execute.c	`88.27% <0.00%> (-0.77%)`	⬇️
src/vim9expr.c	`90.39% <71.42%> (-1.05%)`	⬇️
src/vim9class.c	`88.57% <88.40%> (-1.47%)`	⬇️
src/eval.c	`91.08% <100.00%> (-1.39%)`	⬇️
src/vim9compile.c	`90.46% <100.00%> (-0.81%)`	⬇️

... and 137 files with indirect coverage changes

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 13, 2023, 11:42:40 PM9/13/23

to vim/vim, Push

@yegappan pushed 1 commit.

5d3cee7 Update error message

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

errael

unread,

Sep 14, 2023, 12:17:44 AM9/14/23

to vim/vim, Subscribed

Just ran a few quick tests.

C extends B extends A. In class C, using A.svar, can read/write private members of A, tried that because I remembered you could do that before. Some might consider that a bug. Not sure what I think.

Looking good. I'll dig in more tomorrow.

Messages, I'm fine with the way they are, but some things that occurred to me.

In C, svar get message Class member "svar" accessible only inside class "A",
wondering about working use A.svar and/or from class "A" in there somewhere.
With var oa = A.new() and doing oa.sval get ... accessible only using class "A"
wondering about ... only using A.svar"
With var oc = C.new() and oc.sval get the old favorite Member not found on object
wondering, since sval is in C's kind of parentage, if a message like in 1. is useful.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 14, 2023, 9:21:01 AM9/14/23

to vim/vim, Push

@yegappan pushed 1 commit.

551d524 A readonly class variable can be modified only inside the class where it is defined

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 14, 2023, 9:23:43 AM9/14/23

to vim/vim, Subscribed

Just ran a few quick tests.

Thanks for testing the PR.

C extends B extends A. In class C, using A.svar, can read/write private members of A, tried that because
I remembered you could do that before. Some might consider that a bug. Not sure what I think.

I have updated the PR with a fix for this.

Looking good. I'll dig in more tomorrow.

—

Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 14, 2023, 10:29:24 AM9/14/23

to vim/vim, Push

@yegappan pushed 1 commit.

13c1df0 Add additional tests

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

errael

unread,

Sep 14, 2023, 1:32:04 PM9/14/23

to vim/vim, Subscribed

I played around with vim including this PR. While trying stuff, ran into #13041 (comment); doesn't have anything to do with this PR.

Unless there's something particular you'd like explored, I'll wait for the merge.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 14, 2023, 1:38:17 PM9/14/23

to reply+ACY5DGF67IWP3EDVOD...@reply.github.com, vim...@googlegroups.com, Subscribed, vim/vim

On Thu, Sep 14, 2023 at 10:32 AM errael <vim-dev...@256bit.org> wrote:

I played around with vim including this PR. While trying stuff, ran into #13041 (comment); doesn't have anything to do with this PR.

Unless there's something particular you'd like explored, I'll wait for the merge.

Thanks. Did you see any issues with using class variables and methods in script context, def function context, derived class, another class, using an instance, instance method, class method and different access controls? I know this is a long list. But there are too many scenarios where a class variable and method can be used.

Thanks,

Yegappan

vim-dev ML

unread,

Sep 14, 2023, 1:38:33 PM9/14/23

to vim/vim, vim-dev ML, Your activity

On Thu, Sep 14, 2023 at 10:32 AM errael ***@***.***> wrote:

> I played around with vim including this PR. While trying stuff, ran into #13041
> (comment)

> <https://github.com/vim/vim/discussions/13041#discussioncomment-7004644>;

> doesn't have anything to do with this PR.
>
> Unless there's something particular you'd like explored, I'll wait for the
> merge.
>

Thanks. Did you see any issues with using class variables and methods in
script context, def function context, derived class, another class, using
an instance, instance method, class method and different access controls?
I know this is a long list. But there are too many scenarios where a class
variable and method can be used.

Thanks,
Yegappan

>

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

errael

unread,

Sep 14, 2023, 9:20:46 PM9/14/23

to vim/vim, vim-dev ML, Comment

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. I started putting the test code here, then checked, it's not caused by this PR.

#13041 (comment)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Yegappan Lakshmanan

unread,

Sep 15, 2023, 12:23:36 AM9/15/23

to vim/vim, vim-dev ML, Push

@yegappan pushed 1 commit.

348833b Duplicate class and object methods are ignored

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 15, 2023, 12:24:12 AM9/15/23

to vim/vim, vim-dev ML, Comment

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. I started putting the test code here, then checked, it's not caused by this PR.

I have updated the PR with a fix for this issue also.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

errael

unread,

Sep 15, 2023, 12:42:26 AM9/15/23

to vim/vim, vim-dev ML, Comment

Did you take a peek at #13041 (comment)? _F() in one class and F() in a child or base class gets no error.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Yegappan Lakshmanan

unread,

Sep 15, 2023, 1:40:38 AM9/15/23

to vim/vim, vim-dev ML, Push

@yegappan pushed 1 commit.

0b35ab1 Access level of an object method can be changed in a subclass

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 15, 2023, 1:44:22 AM9/15/23

to vim/vim, vim-dev ML, Comment

Did you take a peek at #13041 (comment)? _F() in one class and F() in a child or base class gets no error.

This is now addressed in the updated PR.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Yegappan Lakshmanan

unread,

Sep 15, 2023, 9:36:07 AM9/15/23

to vim/vim, vim-dev ML, Push

@yegappan pushed 1 commit.

ff948fd Update tests

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

errael

unread,

Sep 15, 2023, 10:10:10 AM9/15/23

to vim/vim, vim-dev ML, Comment

Haven't found any more bugs with this PR (original comment went to wrong spot)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Yegappan Lakshmanan

unread,

Sep 15, 2023, 10:15:56 AM9/15/23

to vim/vim, vim-dev ML, Push

@yegappan pushed 1 commit.

51d95d6 Use the new CheckSourceFailure() function instead of the CheckScriptFailure() function in the tests

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 15, 2023, 10:29:23 AM9/15/23

to vim/vim, vim-dev ML, Push

@yegappan pushed 1 commit.

8eb416d Fix test failure

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

Yegappan Lakshmanan

unread,

Sep 15, 2023, 10:31:26 AM9/15/23

to vim/vim, vim-dev ML, Comment

Haven't found any more bugs with this PR (original comment went to wrong spot)

Thanks for testing the PR.

—

Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Christian Brabandt

unread,

Sep 15, 2023, 2:20:49 PM9/15/23

to vim/vim, vim-dev ML, Comment

Closed #13086 via c30a90d.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Christian Brabandt

unread,

Sep 16, 2023, 7:20:45 AM9/16/23

to vim/vim, vim-dev ML, Comment

Coverity complains about this part here:

*** CID 1544700:  Control flow issues  (DEADCODE)
/src/vim9class.c: 1114 in add_classfuncs_objmethods()
1108                if (loop == 1 && STRNCMP(
1109                                    extends_cl->class_class_functions[i]->uf_name,
1110                                    "new", 3) == 0)
1111                    ++skipped;
1112                else
1113                {
>>>     CID 1544700:  Control flow issues  (DEADCODE)
>>>     Execution cannot reach the expression "extends_cl->class_class_functions" inside this statement: "pf = (loop == 1) ? extends_...".
1114                    ufunc_T *pf = (loop == 1
1115                                            ? extends_cl->class_class_functions
1116                                            : extends_cl->class_obj_methods)[i];
1117                    (*fup)[gap->ga_len + i - skipped] = copy_function(pf);
1118
1119                    // If the child class overrides a function from the parent

—
Reply to this email directly, view it on GitHub.

You are receiving this because you commented.

Yegappan Lakshmanan

unread,

Sep 16, 2023, 10:32:17 AM9/16/23

to vim...@googlegroups.com, reply+ACY5DGGWAC25AGL45K...@reply.github.com, vim/vim, vim-dev ML, Comment

Hi Christian,

On Sat, Sep 16, 2023 at 4:20 AM Christian Brabandt <vim-dev...@256bit.org> wrote:

Coverity complains about this part here:

I have opened the PR https://github.com/vim/vim/pull/13103 to fix this.

Regards,

Yegappan

vim-dev ML

unread,

Sep 16, 2023, 10:32:35 AM9/16/23

to vim/vim, vim-dev ML, Your activity

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Sep 16, 2023, 11:48:43 AM9/16/23

to vim...@googlegroups.com

On Sa, 16 Sep 2023, Yegappan Lakshmanan wrote:

> Hi Christian,
>
> On Sat, Sep 16, 2023 at 4:20 AM Christian Brabandt <vim-dev...@256bit.org>
> wrote:
>
>
> Coverity complains about this part here:
>
>
> I have opened the PR https://github.com/vim/vim/pull/13103 to fix this.

Thanks!

Best,
Christian
--
The steady state of disks is full.
-- Ken Thompson

Reply all

Reply to author

Forward