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

Place parameter check up top. Prevents varargs, but is more robust. #1107

Closed
Dunbaratu opened this issue Jul 22, 2015 · 10 comments
Closed

Place parameter check up top. Prevents varargs, but is more robust. #1107

Dunbaratu opened this issue Jul 22, 2015 · 10 comments
Assignees
Labels
Breaks KSM A VM or opcode change means the user must recompile KSM files. bug Weird outcome is probably not what the mod programmer expected.

Comments

@Dunbaratu
Copy link
Member

@gisikw posted an issue, #1100 which is closed, but I'd still like to address an issue connected to it.

The problem is that since we don't check number of arguments until the bottom of the function, users typically end up seeing complaints about wrong argument types before they see a complaint about mismatched arguments, because passing the wrong number of args causes stack misalignment. (Which IS detected eventually, but not until after it tried using the values that were shifted into the wrong variables, which often causes its own errors that stop it before it gets to the check for missing or too many args).

The entire reason for not checking until the bottom was to allow for clever varying args situations where you execute the declare parameter commands conditionally depending on what you see.

But I doubt anyone is doing this, and it's preventing us from giving proper error messages out for mismatched argument counts.

I'd like to switch it to a system that requires the declare parameter commands to be "flat" in the outermost nesting level (not contained inside loops or if's), and can throw an error right away when they don't have the right number of arguments to match. I might provide people with an alternate technique where they can get a LIST() of parameters instead if they like to try some vararg logic.

@Dunbaratu Dunbaratu self-assigned this Jul 22, 2015
@gisikw
Copy link
Contributor

gisikw commented Jul 23, 2015

Just curious here on the latter part. Are you referring to setting up an external mechanism to make passing LIST()s to functions easier? Or adding something akin to the JavaScript arguments pseudo-array?

Can definitely think of some uses for the latter (particularly passing "flags" to functions, which couldn't be done using conditional declares). Either way, excited for this :)

If I can feature creep a tiny bit...would there be any possibility of allowing functions to be passed as arguments (either directly or through a &myFunction syntax or something). Right now, they're automatically executed whenever they're referenced. But if we had higher-order functions, we'd be able to create user pseudostructs in KerboScript!

@Dunbaratu
Copy link
Member Author

I was considering a means of varargs that make a list for you, not something where you just pass a list, which you can do now. The idea is that it would get the remaining args up to the end, like so:

function foo {
  parameter x,y.
  parameterlist z.
  // .. body here
}

foo( 1,2,3,4,5,6,7).

That would fill x with 1, y with 2, and z with list(3,4,5,6,7).

A parameterlist would have to come last, only allowing varying args at the end.

As for function pointers, it's definitely a doable thing if we just add the syntax for it. Right now we don't just because it wasn't a high priority.

@gisikw
Copy link
Contributor

gisikw commented Jul 23, 2015

Oh nice, so just a trailing splat. I like it!

@hvacengi
Copy link
Member

Do you think that we could also turn the system into named parameters once @erendrake's associative array is finished and merged? Maybe using parameterlex instead? Then running the function you could do dosomething(abc: 0, xyz: 100, lmn: 50). That would allow you to use optional parameters that don't necessarily have to be dependent only on the raw number of parameters. For example, say you have a total of 10 parameters that can be passed to a function. If you want to skip both the 2nd and the 8th parameter one time, and both the 3rd and the 7th another time, a lex would give the option of checking "has key" and being able to check which parameters were skipped, while the list forces you to add another parameter telling it which mode you want to use. Parameters passed without a label could simply use an index number in it's place.

@gisikw
Copy link
Contributor

gisikw commented Jul 23, 2015

That feels like a significant diversion in language style. It feels like you'll be easily able to do that with dosomething({abc: 0, xyz: 100, lmn: 50}, param2), or whatever the hashdict syntax ends up being.

@hvacengi
Copy link
Member

Good point. Just pass the lex as a parameter itself... Why did I want to make that more complicated?

@Dunbaratu
Copy link
Member Author

Named args is a good language feature, but it's quite a departure from how kerboscript works at the moment.

@Dunbaratu
Copy link
Member Author

So I think I'm going to make a hidden user-land LIST() variable with local function scope that holds all the parameters and gets filled up at the top before you do anything, having popped the stack all the way off, removing all the parameters. Then whenever you do parameter foo, it's just going to eat the head of the list and cut it down. When you hit parameterlist, it will give you what's left in the list. Thus you can't get stack misalignment because it already consumed everything before you even began.

@Dunbaratu Dunbaratu added bug Weird outcome is probably not what the mod programmer expected. Breaks KSM A VM or opcode change means the user must recompile KSM files. labels Aug 24, 2015
@Dunbaratu
Copy link
Member Author

(Added "Beaks KSM" as a reminder that whatever solution is used, it will change the way the args have to be processed in the stack, which does invalidate old KSM files that had function calls in them.)

@erendrake
Copy link
Member

Fixed by #1133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaks KSM A VM or opcode change means the user must recompile KSM files. bug Weird outcome is probably not what the mod programmer expected.
Projects
None yet
Development

No branches or pull requests

4 participants