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

kwargs error in recent py/runtime commits #8473

Closed
stephanelsmith opened this issue Mar 31, 2022 · 9 comments
Closed

kwargs error in recent py/runtime commits #8473

stephanelsmith opened this issue Mar 31, 2022 · 9 comments
Labels

Comments

@stephanelsmith
Copy link
Sponsor Contributor

stephanelsmith commented Mar 31, 2022

Updating to master, noticed a new issue relating to kwargs. Last working commit is bb70874. On commit 1e99d29, I get a core dump, and 783b1a8 onwards, I get a TypeError.

I'm running IDF v4.4. I'm running this on ESP S3.

ERROR 2e3f204 (HEAD -> master, origin/master, origin/HEAD) py/runtime: Use size_t/ssize_t instead of uint/int.
4768518 tests/basics/fun_callstardblstar: Add coverage test.
9b74d71 py/runtime: Drop new_alloc < 4 check.
3679a47 py/runtime: Do not overallocate when len is known.
ERROR 783b1a8 py/runtime: Allow multiple *args in a function call.
LoadProhibited 1e99d29 py/runtime: Allow multiple **args in a function call.
PASS bb70874 py/vm: Prevent array bound warning when using -MP_OBJ_ITER_BUF_NSLOTS.

Here's the test case. This schtick on this one is direct instantiation of kwargs is working fine. Which is why it probably passed test cases (and why this was a little tricky to find). But when you pass kwargs through a function, I see the issue.

def kwargs_test1(kwargs):
    KwargsTest(**kwargs)
class KwargsTest():
    def __init__(self, hello            = None,
                       world            = False,
                       ):
        pass

kwargs = {'hello':True}

#direct instantiation works across commits
KwargsTest(**kwargs)
# <KwargsTest object at 3d832510>

#calling via function causes error on new commits
kwargs_test1(kwargs=kwargs)
# on commit bb7087411 and earlier, no error.
# on commit 1e99d29f3 core panic
# on commit 783b1a868 and owwards, I get this:
# Traceback (most recent call last):
  # File "<stdin>", line 1, in <module>
  # File "main.py", line 189, in kwargs_test1
# TypeError: can't convert dict to int


@dlech
Copy link
Sponsor Contributor

dlech commented Mar 31, 2022

Is the code compiled with mpy-cross first?

If the answer is yes, then you need to build mpy-cross from the matching commit since bytecodes changed.

If the answer is no, do the proposed changes in #8472 fix the problem?

@stephanelsmith
Copy link
Sponsor Contributor Author

stephanelsmith commented Mar 31, 2022

On master branch, ran at root
make -C mpy-cross

Rebuilding, now I a build error

Traceback (most recent call last):
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 1609, in <module>
    main()
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 1584, in main
    compiled_modules = [read_mpy(file) for file in args.files]
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 1584, in <listcomp>
    compiled_modules = [read_mpy(file) for file in args.files]
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 1262, in read_mpy
    raw_code = read_raw_code(reader, cm_escaped_name, qstr_table, obj_table, segments)
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 1215, in read_raw_code
    read_raw_code(reader, cm_escaped_name, qstr_table, obj_table, segments)
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 1191, in read_raw_code
    rc = RawCodeNative(
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 913, in __init__
    super(RawCodeNative, self).__init__(
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 735, in __init__
    ) = extract_prelude(self.fun_data, prelude_offset)
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 423, in extract_prelude
    ) = read_prelude_sig(local_read_byte)
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 371, in read_prelude_sig
    z = read_byte()
  File "/home/ssmith/micropython/tools/mpy-tool.py", line 411, in local_read_byte
    b = bytecode[ip_ref[0]]
IndexError: index out of range

Will look at proposed change here shortly. Was error reproducable on your end?

@dlech
Copy link
Sponsor Contributor

dlech commented Mar 31, 2022

Was error reproducable on your end?

No, I copied and pasted the example from the first comment and ran it in a unix 64-bit build without error.

@dlech
Copy link
Sponsor Contributor

dlech commented Mar 31, 2022

The mpy-tool.py error could be due to other recent changes like #8191 or #8433.

@stephanelsmith
Copy link
Sponsor Contributor Author

stephanelsmith commented Mar 31, 2022

Yeah, I recently was looking at which turned out to be a byte code issue, #8458
Trying to get revert back to last working mpy-cross now...

Could very well be related if there was additional byte code format change between 35dbde1 and the latest commit.

Will keep looking for esp32.

@stephanelsmith
Copy link
Sponsor Contributor Author

stephanelsmith commented Mar 31, 2022

I'm able to re- "make -C mpy-cross" and build without error from this commit
35dbde1
Is fairly new, only 4 days old. Why it's not working on latest sounds like a confounding/contriubting factor.

@dpgeorge
Copy link
Member

dpgeorge commented Apr 1, 2022

Please can you re-try with latest master, now that #8472 is merged. If it still fails, please give exact steps to reproduce (and try without any @micropython.native).

@stephanelsmith
Copy link
Sponsor Contributor Author

Confirming that with all @micropython.native commented out

  • Build error above goes away
  • The weird kwargs issue also goes away

Looks like @micropython.native is it!

Additional info:

  • Pulled master (8baf05a (HEAD -> master, origin/master, origin/HEAD) py/makeqstrdefs: Cleanup and extend source file classification.)
  • Re-made mpy-cross, clean build.
  • Running on custom S3 board

Finally, (getting ahead of myself), @micropython.native I've never got working on async functions. I don't think it's an "issue" but just wanted to put it on the radar!

I'll keep working on master and comment out all @micropython.native which is fine for me. I'll close both issues on my end here too.

@stephanelsmith
Copy link
Sponsor Contributor Author

Also, @dlech and @dpgeorge huge thank you . This is tricky business. I sincerely appreciate your taking up my questions and your patience as I work through on my end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants