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.
CPython cannot create a second empty string: PyUnicode_New(0) returns
_PyUnicode_GetEmpty() (Objects/unicodeobject.c:1281 in the 3.15 tree), so
`x is ''` holds for every empty result of every operation. We allocated a
fresh 112-byte husk, which is both a whole string lifecycle spent and an
identity difference a user can see:
'a,,b'.split(",")[1] is '' CPython True ours was False
`ascii_piece` is the builder behind split/rsplit-with-ascii-seps/partition, and
it already special-cased single-byte pieces out of the latin-1 cache, so the
empty case joins it in one line. That turns the three split rows green; the
other ten red rows (join, the case/strip family, format, str(), the right-
hand partition variants) come from different builders and stay open in task
RustPython#100, which is why bench/labs/test_empty_str.py pins 23 named rows rather than
a single smoke test - the file is the coverage map for the remaining work.
Measured on bench.py across two release-lite binaries of the same tree, both
orientations: geomean 0.996 and 1.005, i.e. neutral. The change is banked for
the parity, not claimed as a speed win; the win would only show on shapes that
actually produce empty pieces. Gate: verify.ps1 over 16 modules on the new
binary, exit=0, and the probe's own 13 -> 10 failures.
Also in this commit, two instruments and one correction to a probe:
- bench/labs/bitchurn.py exists because bench.py has twelve cases and none of
them allocates a bytes object, so a bytes-model change is unmeasurable there.
Using it, the slim header for PyBytes came out +7.1 % on `bytes encode` but
~1.0 on the geomean of the bytes shapes with the control row drifting +0.4 %,
so that experiment was reverted rather than banked.
- str_object_lifecycle_cost now runs to length 40 and not just 4/20. SSO
capacity is 22, so the old list described only strings whose bytes never
reach the allocator, which is why it could not see the second malloc a
>22-byte string pays. Its doc now also states that these ns are the Windows
system allocator's, because mimalloc is installed only in the binary, so the
numbers must not be compared against a bench.py case.
Assisted-by: Qoder Agent:claude-opus-5
This allows defaults to be specified at function definition: