-
Notifications
You must be signed in to change notification settings - Fork 215
Add default-parameters feature. #1054
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
base: main
Are you sure you want to change the base?
Conversation
|
Wow, I'll need some time to look thru this, but great work! Actually a lot of people have asked for it, but I'm not quite sure named arguments is a good idea. Currently default values can be simulated by have multiple definitions of the same function with different parameters. How is it going to work in this case: fn foo(x, y, z = 1) {}
fn foo(x, y) {}
foo(0, 1); // which version is called? |
|
@schungx to answer your question, the second one would be called. But that case unfortunately poke at a fragile point of the code. It doesn't really play well with function overloading. While playing with your question I ran into this here: repl> fn foo(x,y,z=1) {print("first")}
repl> fn foo(x,y) {print("second")}
repl> foo(1,1)
second
repl> foo(1,2,3)
first
repl> foo(1,2,z=3)
foo(1,2,z=3)
^ Function not found: foo (unknown named argument 'z')I feel this is not acceptable, wdyt? The call code is not the easiest to work with, but I will try to figure out whats going on there. |
|
@schungx I fixed the issue, from my perspective it does now what it should. User defined functions that overload a function name and match from the parameters are now always preferred over functions that match because of default parameters. |
|
I went wild and dropped the requirement that the defaults are just Dynamics. Instead now any Expression is supported and it's even possible to access previously defined arguments like in |
I assume you enhance the function resolution process by searching for default parameters after searching for I can think of a scenario: when I call func with two arguments, Rhai searches for functions with arity two. If it failes, does it then search for functions of arity three with one default parameter? How about four with two defaults? When do we stop? And how do you precalculate the call hashes for so many arities? Finally I suppose it doesn't work with Also, I see that you haven't changed |
|
Yes, when no direct hit for a given name and arity is found, we search for functions with a greater arity. Currently it searches up to N+10 parameters, where N is 2 in your case. This is quite arbitrary, and perhaps it would make sense to crank this up to 32. |
|
Looking at this deeper, it seems like too ambitious a change at this point. In particular, I am concerned about named arguments. They easily make a scripting API very brittle by hard-coding parameter names into all call sites. That exposes the internal structure of the function to public and thus make functions less self-contained (because parameter names are now part of the signature). Such a function is extremely costly to refactor... you change a parameter name, you possibly change ALL scripts that call this function. And it makes defining functions in Rust problematic because we must now provide all the parameter names... Default parameters essentially make function arity variable... so the function's arity is no longer a defining factor, and it causes a caller/callee arity mismatch that would require manual searching to match. So you can't pre-calculate hashes for them. That means that all functions that use default parameters are automatically put on the "slow-path". |
|
I also register functions with default parameters multiple times under hashes with all valid parameter counts. At some point I tried removing the guessing (that counting up to 10), but then some tests blew up and I kept it that way. Theoretically we should get a direct hit for a function with default parameters. What do you mean with call_fn()? |
You can |
|
By the way, do you use Discord? Would be much easier for a conversation if we open up a thread inside the chats. |
|
I don't have discord, but you can find me on telegram under the same handle @trusch. I was able to remove the guessing code (that counting up to 10) because when making the function overloading right I somehow fixed the problem that was causing the need for this accidentally. So now we don't guess, we either find the function or we don't in the first lookup. Good catch with the qualified function calls, fixed it. Regarding the named arguments being a bad idea: I see your point that the argument names then become a part of the signature and that this causes more coupling between the function and the caller. But this is nothing bad inherently. Function developer and consumer just need to be aware of the fact, that changing parameter names may have consequences. After all its an opt-in feature, so when you as a developer of some scripting environment activates that feature, you also opt-in for this "restriction". |
How did you do it? If I have I was concerned with the fact that all functions with default parameters are on the slow path, which may not be the case if you don't do an exhaustive search. |
I'd suggest separating the default parameters features with the named parameters feature, and make them separate branches. Because they are really two different language features. Named parameters can be supported even without default parameters, and vice versa. |
When registering the functions in module/mod.rs, I register them for each valid parameter count. So for I'm not sure splitting the two features really make sense. I get where you are coming from, but default parameters alone don't make much sense if you can't leave out some of the defaults when calling and then specifying a later one. Like in Lets have a more plastic example. fn create_sound(freq = 800., amp = 0.2, release = 0.1) { ... }
// without named parameters
create_sound(); // 800 0.2 0.1
create_sound(900.); // 900 0.2 0.1
create_sound(800, 0.3) // 800 0.3 0.1
create_sound(800, 0.2, 0.2) // 800 0.2 0.2
// with named parameters
create_sound() // 800 0.2 0.1
create_sound(freq = 900.) // 900 0.2 0.1
create_sound(amp = 0.3) // 800 0.3 0.1
create_sound(release = 0.2) // 800 0.2 0.2I feel this interacts really nicely and basically only makes sense together from my perspective. Without it the default parameters would just be syntactic suggar for function overloading |
Ah. Gotcha. In that case, assuming we'd support only literals for default values (not expressions at this time), then it would actually be a relative small change to the code base to support this feature. Essentially it would almost seem like we're registering a curried function instead of a standard function.
It does. Because in many practical use cases, it is often to have one optional parameter or some sort of a flag/config that can be left out. For example, a Having more then one optionals, we're really talking about a "property bag". In which case it would be much easier to pass in a map named |
|
Revisiting this... I am still of the opinion that we should split default parameters from named parameters. The named parameters can be refactored this way, which is actually more JavaScript-y: fn create_sound(first_arg) {
if type_of(first_arg) == "map" {
make_sound(freq.freq ?? 800, freq.amp ?? 0.2, freq.release ?? 0.1)
} else {
make_sound(first_arg, 0.2, 0.1)
}
}
fn create_sound(freq = 800, amp = 0.2, release = 0.1) {
make_sound(freq, amp, release)
}
// with named parameters
create_sound() // 800 0.2 0.1
create_sound(900); // 900 0.2 0.1
create_sound(800, 0.3) // 800 0.3 0.1
create_sound(800, 0.2, 0.2) // 800 0.2 0.2
create_sound(#{ freq: 900 }) // 900 0.2 0.1
create_sound(#{ amp: 0.3 }) // 800 0.3 0.1
create_sound(#{ release = 0.2 }) // 800 0.2 0.2
create_sound(#{ freq: 900, amp: 0.3, release = 0.2 }) // 900 0.3 0.2 |
Add Default Parameters and Named Arguments
This PR introduces default parameter values for function definitions and named argument support when calling functions. This feature is gated behind the
default-parametersfeature flag. I wanted this for a personal project and wasn't able to figure this out any other way than adding it directly to rhai.Features
Default Parameter Values
Functions can now define default values for parameters using the
=syntax:Expression Defaults
Default parameter values can be any expression, including references to previous parameters, function calls, constants, and complex expressions:
Named Arguments
When calling functions, you can specify arguments by name, allowing you to skip parameters with defaults:
Closures with Expression Defaults
Closures support default parameters and can capture outer scope variables in their default expressions:
Supported Use Cases
let f = |a, b = 2| { a + b }fn add(a = 1, b = 2) { a + b }thisin closure defaults for method-like behaviorSyntax Rules
Positional arguments must come before named arguments:
Arguments cannot be provided both positionally and by name:
All required parameters must be provided:
Named arguments must match actual parameter names:
Default expressions are evaluated at runtime for each function call, allowing them to depend on previously evaluated parameters and the current execution context.
Implementation Details
default-parameters(opt-in)Testing
Comprehensive test suite included in:
tests/default_params.rs- Basic default parameters, named arguments, error casestests/default_expr.rs- Expression defaults, parameter dependencies, closures, complex expressionsTest coverage includes:
thisin closure defaultsExample Usage