Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow overload on extern methods on all targets #9793

Merged
merged 2 commits into from
Aug 3, 2020
Merged

Conversation

Simn
Copy link
Member

@Simn Simn commented Aug 1, 2020

This allows using overload methods which are marked extern or live on extern classes. The idea is that we want to allow this as long as we don't have to worry about generating the overloaded methods on targets that have no native support for it.

Some notes:

  • Overloaded constructors are explicitly disallowed
  • There are no other restrictions in place because the relevant restrictions should follow from the extern status
  • @:overload is not supported.
  • I'm adding one .disabled test which checks the presence of override. We currently don't run any checks like that on externs at all, so this may or may not be something to look into.

There are now only 4 checks for pf_overload left:

  1. @:overload handling
  2. Admit overloads for all methods, not just externs
  3. Handle overloaded constructors
  4. Don't call add_constructor because it doesn't support overloads

While the first two should remain in place, we could at some point look into supporting overloaded constructors as well. This can first be done for true externs, and maybe even extended to extern inline constructors afterwards.

I'm sure there are some missing checks here, but we can add those when we come across them.

@skial skial mentioned this pull request Aug 3, 2020
1 task
@Simn Simn merged commit 975b587 into development Aug 3, 2020
@Simn Simn deleted the overload_approach branch August 3, 2020 20:25
jdonaldson pushed a commit that referenced this pull request Sep 3, 2020
* [typer] lose some more pf_overload checks

* [typer] enable overloads
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.

2 participants