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

Rest arguments #9274

Conversation

RealyUniqueName
Copy link
Member

@RealyUniqueName RealyUniqueName commented Mar 26, 2020

Closed in favor of #9961

Implements rest arguments as an abstract over targets native corresponding features.
Closes #7306
Softly deprecates haxe.extern.Rest (no warning for now).

  • js
  • python
  • php
  • flash
  • cpp
  • eval
  • lua
  • java
  • cs
  • jvm
  • neko

@RealyUniqueName
Copy link
Member Author

@jdonaldson Could you please add a lua implementation?

import haxe.iterators.RestIterator;

@:coreType
abstract Rest<T> {
Copy link
Member

Choose a reason for hiding this comment

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

to Iterable<T> for static extension support might be nice?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, because it's not compatible with Iterable.
The only thing we can to is adding @:to Iterable

Copy link
Member

Choose a reason for hiding this comment

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

Hm... but it has iterator()?

Copy link
Member Author

Choose a reason for hiding this comment

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

But it only exists at compile time, while Iterable needs iterator to exist at run time.

@jdonaldson
Copy link
Member

Looks like a good start... I got things working for Lua in your test cases.

I think we'll need to add some more restrictions for Rest.... here's some things that are possible, but add a lot of complications to support:

  1. Returning a Rest type might need special behavior
  2. Passing a Rest type as a dynamic argument.

These should probably be restricted, with a helpful compiler message:

  1. Passing a Rest type as an argument before other standard arguments.
  2. Passing two or more Rest types

I'm not handling any of these cases yet

@RealyUniqueName
Copy link
Member Author

  1. Passing a Rest type as an argument before other standard arguments.
  2. Passing two or more Rest types

These are handled by the compiler

std/lua/_std/haxe/Rest.hx Outdated Show resolved Hide resolved
@jdonaldson
Copy link
Member

jdonaldson commented Mar 27, 2020

  1. Passing a Rest type as an argument before other standard arguments.
  2. Passing two or more Rest types

These are handled by the compiler

There's not a very clear/explicit error message for these cases...

function rest(r:Rest<Int>, o :Int){
  return o;
}
[...]
var r = rest(3, 2, 1, 0);

or

function rest(r:Rest<Int>, o : Rest<Int>){
  trace(r + " is the value for r");
  return o;
}

Gives:

Main.hx|8 col 16 error| Int should be haxe.Rest<Int>
Main.hx|8 col 16 error| For function argument 'r'

@RealyUniqueName
Copy link
Member Author

There's not a very clear/explicit error message for these cases...

Looks like it's only checked for fields. Will add for local functions too.

import lua.TableTools.maxn;
import lua.TableTools.pack;
import lua.Table;

Copy link
Member

Choose a reason for hiding this comment

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

Spaces -> Tabs again, to be consistent with the stdlib.

Copy link
Member

Choose a reason for hiding this comment

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

Or just run haxe-formatter over it, there's other formatting issues too. :D

Copy link
Member

Choose a reason for hiding this comment

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

FWIW I added haxe-formatter to this vim plugin : vim-autoformat/vim-autoformat#297

I have it set to autorun when I open a haxe file now.

@RealyUniqueName RealyUniqueName added this to the Backlog milestone Mar 30, 2020
@skial skial mentioned this pull request Mar 31, 2020
1 task
@jdonaldson
Copy link
Member

Should we support returning a Rest argument? I added a breaking test for it.

@nadako
Copy link
Member

nadako commented Mar 31, 2020

So... does this support calling rest-argument functions by passing Array (or something that can be created from one) as the rest argument? (i.e. exec(a, b, myRestArgs)). This would be required for such a feature because we have inheritance and super calls (also constructors).

@RealyUniqueName
Copy link
Member Author

So... does this support calling rest-argument functions by passing Array (or something that can be created from one) as the rest argument?

Not yet. I have to figure out inlining for this case first. Or disable inlining functions with rest args completely.

@Simn
Copy link
Member

Simn commented Jul 28, 2020

This PR will need an update.

Also, we have to stop randomly stopping to work on things like these.

@RealyUniqueName
Copy link
Member Author

This was not planned and I've started this because I thought it wouldn't take a lot of effort, but it turned out I was wrong, so I moved back to planned tasks.

Main issue is an absence of syntax for passing a collection as rest arguments (e.g. callWithRest(...args))

@ALANVF
Copy link
Contributor

ALANVF commented Aug 22, 2020

Curious why Neko isn't supported so far. It supports variadic functions and dynamic function application, no?

@RealyUniqueName
Copy link
Member Author

Closed in favor of #9961

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.

[Feature Request / Lua] Type for handling Lua var args "..."
7 participants