Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: support 'this' type on methods and properties#2906

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
mattjohnsonpint wants to merge5 commits intoAssemblyScript:main
base:main
Choose a base branch
Loading
frommattjohnsonpint:issue-2904

Conversation

mattjohnsonpint
Copy link
Contributor

@mattjohnsonpintmattjohnsonpint commentedFeb 5, 2025
edited
Loading

Fixes#2904

Changes proposed in this pull request:

  • Fixes support for polymorphicthis type on class methods and properties
  • Reports a "not implemented" forfields declared with thethis type (would be nice, but is complicated)
  • Adds compiler tests for this feature

To clarify, thethis type wasalready implemented, but was returning the base type instead of the calling instance type.

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

snydergd reacted with heart emoji
@mattjohnsonpintmattjohnsonpint marked this pull request as draftFebruary 5, 2025 23:43
@mattjohnsonpintmattjohnsonpint marked this pull request as ready for reviewFebruary 6, 2025 01:07
@mattjohnsonpint
Copy link
ContributorAuthor

FYI, I considered trying to bind the prototype instance to the derived type, but I couldn't quite get that way to work. So instead, I resolve the current "this" to use as the class instance instead. It works, but let me know if there's a better way to approach this. Thanks.

Copy link
Member

@HerrCai0907HerrCai0907 left a comment

Choose a reason for hiding this comment

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

Could we create an override method for inheritance class
e.g.
forclass A extends B, B has methodfoo returnthis, we resolve it as returnB. A will generate override methodfoo returnA.
Then resolve function should resolve to correct function.

@mattjohnsonpint
Copy link
ContributorAuthor

Could we create an override method for inheritance class ...

I explored this a bit. It would have side effects, even without using thethis return type.

For example, given:

classX{privatefoo:i32=0;doSomething():void{this.foo=1;}}classYextendsX{}

... we can't just transform it to this:

classX{privatefoo:i32=0;doSomething():void{this.foo=1;}}classYextendsX{doSomething():void{this.foo=1;}}

... because fieldfoo is declared private.

Theresolver implementationdoes effectively create an alias of the member, but in doing so it uses the original prototype, bound to the base instance. It doesn't create an actual override.

@github-actionsGitHub Actions
Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions!

@snydergd
Copy link

@HerrCai0907 does@mattjohnsonpint make sense in their explanation? Is there anything else to consider? I think it would be great to get support for this feature.

@github-actionsGitHub Actions
Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions!

@snydergd
Copy link

I am wondering if there are still any outstanding concerns with this or if it could be merged.

Comment on lines +2871 to +2872
// Note: It should work without this outer type check, and does when testing, but fails the "bootstrap" build.
// TODO: Figure out why and remove the extra check.
Copy link
Member

Choose a reason for hiding this comment

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

Could you analyze more detail about bootstrap? What is the meaning of "outer type check"?

@MaxGraeyMaxGraey removed the stale labelJun 13, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@HerrCai0907HerrCai0907HerrCai0907 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

this type should return instance type when overridden
4 participants
@mattjohnsonpint@snydergd@HerrCai0907@MaxGraey

[8]ページ先頭

©2009-2025 Movatter.jp