Repository navigation
Add support for function argument defaults - #100
Conversation
| } | ||
| } | ||
|
|
||
| bitflags! { |
There was a problem hiding this comment.
I think I would go for an extra boolean in the function: has_defaults. So no extra packages.
There was a problem hiding this comment.
This will eventually need to carry 4 values (per https://docs.python.org/3/library/dis.html#opcode-MAKE_FUNCTION), which is why I went with the bitflags approach. It also maintains symmetry with the CPython implementation.
If you'd still prefer for me to change it, I'd be happy to do so.
(FWIW, we also already pull bitflags in due to a couple of dependencies, so this has no net impact of our dependency tree.)
There was a problem hiding this comment.
My reasons would be that using booleans would allow the rust compiler to implement bitfields by itself. I think bitfields are an optimization from times where memory was scarce and compilers did not implement this. I would not be surprised if rust under the hood implemented booleans as bitfields.
Leave it as it is for now :)
| for (name, value) in code_object.arg_names.iter().zip(args.args) { | ||
| let nargs = args.args.len(); | ||
| let nexpected_args = code_object.arg_names.len(); | ||
| let args = match nargs { |
There was a problem hiding this comment.
Do not use match in this case, but a sequence like this:
if nargs > expected {
} else if nargs < expected {
} else {
}
| for (name, value) in code_object.arg_names.iter().zip(args.args) { | ||
| let nargs = args.args.len(); | ||
| let nexpected_args = code_object.arg_names.len(); | ||
| let args = match nargs { |
There was a problem hiding this comment.
Do not use match here, but if-elseif-else:
if nargs > expected {
...
} else if nargs < expected {
...
} else {
...
}
|
Nice job! |
The enforcement of the ordering of default/non-default arguments happens in the compiling step, but should perhaps eventually be pushed back in to the parsing phase.
This allows defaults to be specified at function definition: