Skip to content

Compatible getters/setters #401

Merged
Perryvw merged 5 commits intomasterfrom
getters-setters
Feb 17, 2019
Merged

Compatible getters/setters #401
Perryvw merged 5 commits intomasterfrom
getters-setters

Conversation

@tomblind
Copy link
Copy Markdown
Collaborator

fixes #206

This only affects classes that have getters and/or setters. I went with the solution here: #206 (comment)

One edge-case to note is this:

class Foo {
    prop = "foo";
}
class Bar extends Foo {
    get prop() { return "bar"; }
}
print((new Bar()).prop);

Because prop is assigned directly to the instance in the base class, the getter defined in the derived class would never be accessed. My solution to this is to detect this specific case and set the instance's property to back nil in derived class' constructor.

Bar.prototype.____constructor = function(self, ...)
    Foo.prototype.____constructor(self, ...);
    rawset(self, "prop", nil);
end;

Since the base property is effectively inaccessible in this situation, I don't think this should cause any undesirable side-effects.

@tomblind tomblind requested review from Perryvw and lolleko February 14, 2019 20:28
@TestCase("inst.field | 3", 8 | 3)
@TestCase("inst.field << 3", 8 << 3)
@TestCase("inst.field >> 1", 8 >> 1)
@TestCase("inst.field = 3", 7)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test right? We should verify if typescript returns the literal number or the field getter.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was previously returning the wrong value. When an assignment is evaluated, the value on the right is returned. The left should not be evaluated. Playground

- removed special case for numerical literals when comparing property names
- added tests for accessors overriding other accessors
@Perryvw Perryvw merged commit c002783 into master Feb 17, 2019
@Perryvw Perryvw deleted the getters-setters branch February 17, 2019 14:23
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.

Property getters/setters 'lost' when accessed from interface

2 participants