Repository navigation
Conversation
This commit changes "*" to the null qstr for all internal parameter names,
such as closed over variables.
That fixes a subtle bug where passing a dict like `**{"*": 1}` to a closure
would actually match the internal parameter named "*". Now using the null
qstr means that there's no way to match from a dict.
This has a minor change to the output from `mpy-tool.py`, which is now:
args: [None]
instead of previously `args: ['*']`.
Signed-off-by: Damien George <damien@micropython.org>
This commit implements PEP 570: positional-only parameters, which are annotated via a "/" in the function/lambda parameter list; see https://peps.python.org/pep-0570/ The implementation is simple: positional-only parameters are given the name MP_QSTRnull in the prelude, so keyword matching at runtime in `mp_setup_code_state_helper()` does not match against them. This approach gets the same semantics as CPython while keeping the runtime and .mpy format unchanged. Inspecting generated bytecode is the best way to see how this feature works. For the function definition: def f(a, b, c): pass The qstr table in the corresponding .mpy file looks like: qstr_table[6] (* for static qstrs): 'x.py' '<module>' * 'f' 'a' 'b' 'c' and the bytecode for the function `f` is: 1b:0a:02:03:04:05:20:51:63 The `02:03:04:05` bytes are indices into `qstr_table` for the function name and then the three parameter names. Changing that function to use positional-only parameters: def f(a, b, c, /): pass the qstr table is now: qstr_table[4] (* for static qstrs): 'x.py' '<module>' * 'f' None * and the bytecode for `f` is: 1b:0a:02:03:03:03:20:51:63 This is the same number of bytes as before, but the parameter names are the `03:03:03` bytes which index the `None` entry in `qstr_table`. The total .mpy file size is eight bytes smaller here because it no longer stores the three parameter names. The user-facing differences to CPython are minimal: - In MicroPython, `locals()` doesn't know about the positional-only parameter names. But MicroPython already doesn't support local function variables in `locals()`. - Error messages are slightly different. For an example of a different error message, consider executing: def f(a, /): pass f(a=1) CPython gives: TypeError: f() got some positional-only arguments passed as keyword arguments: 'a' whereas MicroPython gives: TypeError: unexpected keyword argument 'a' Signed-off-by: Damien George <damien@micropython.org>
These tests were written by Claude Opus 5.5. Signed-off-by: Damien George <damien@micropython.org>
Frozen code goes through `mpy-tool.py` and that tool needs to correctly handle functions that have positional-only parameters, which requires handling `MP_QSTRnull`. Signed-off-by: Damien George <damien@micropython.org>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19799 +/- ##
=======================================
Coverage 98.59% 98.59%
=======================================
Files 182 182
Lines 23346 23356 +10
Branches 5 5
=======================================
+ Hits 23017 23027 +10
Misses 328 328
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Code size report: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements PEP 570: positional-only parameters, which are annotated via a "/" in the function/lambda parameter list; see https://peps.python.org/pep-0570/
The implementation is simple: positional-only parameters are given the name MP_QSTRnull in the prelude, so keyword matching at runtime in
mp_setup_code_state_helper()does not match against them. This approach gets the same semantics as CPython while keeping the runtime and .mpy format unchanged.This PR is an alternative to #8480. The benefit of this one here is that it does not require a change to the .mpy file format. So code compiled with mpy-cross prior to this PR will work on VM's post this PR.
Inspecting generated bytecode is the best way to see how this feature works. For the function definition:
The qstr table in the corresponding .mpy file looks like:
and the bytecode for the function
fis:The
02:03:04:05bytes are indices intoqstr_tablefor the function name and then the three parameter names.Changing that function to use positional-only parameters:
the qstr table is now:
and the bytecode for
fis:This is the same number of bytes as before, but the parameter names are the
03:03:03bytes which index theNoneentry inqstr_table. The total .mpy file size is eight bytes smaller here because it no longer stores the three parameter names.The user-facing differences to CPython are minimal:
locals()doesn't know about the positional-only parameter names. But MicroPython already doesn't support local function variables inlocals().For an example of a different error message, consider executing:
CPython gives:
whereas MicroPython gives:
Testing
Tests are added to the standard test suite and will be run under CI on all targets (the feature is not optional, so all ports will get it).
Trade-offs and Alternatives
"*"string as well as closures (because now the compiler usesMP_QSTR__star_as well asMP_QSTRnullin the generated mpy data). In the entire micropython-lib library totaling about 600k bytes of .mpy data, the changes here added only +20 bytes to that.Generative AI
I used Claude Opus 5.5 to discuss the problem of implementing PEP 570 and brainstorm some alternative ways of doing it. It came up with the approach here, but I implemented it all by hand. Claude also wrote some of the tests in this PR, and reviewed the commits (finding a few bugs along the way, like the need to use
MP_QSTRnullinstead ofMP_QSTR__star_).