353: Add support `argv` parameter of SPy main function by kanin-kearpimy · Pull Request #449 · spylang/spy

kanin-kearpimy · 2026-04-02T12:58:47Z

Issue: #353

Hi everyone,

this is implementation of backend SPy argv parameter for main function.

I separated argument wrapper to list.c and list.h then calling it in cmodwriter.

However, there is a tricky part happening

if any spy files need argument in main like below

def main(argv: list[str]) ...

It will include list.h to such compiled C file to use spy_wrap_argv. However, such implementation is in list.c which requires to compile before such current working file got compile.

Rough correct pipeline.

1) **pre-compile stage**
_list.c (and other built-in standard lib)

2) **current working files**
-> list.c -> [any current working file with `main(argv: list[str])`]

Our current pipeline does compile all standard libraries (e.g. _list.spy) at once. It's challenging to modify current pipeline to receive customized on-the-fly .c to compile as above pipeline. Also, to compile list.c with all libspy is challenging because list.c need _list.c to compiled first.

So, now I juggled by copy content of list.c and compile it with [any current working file with main(argv: list[str])] instead.

Well, I'm not sure I explained it clearly. Please let me know if you need elaboration.

antocuni

thanks @kanin-kearpimy .

Congrats to find a way to solve the list[str] bootstrapping problem, I didn't think about it and indeed it's annoying.

I think we can solve it in a much easier way by just emitting the code directly in the generated C instead of trying to have it in a separate file.

See me comment below

antocuni · 2026-04-02T16:42:02Z

spy/backend/c/cmodwriter.py

+            returns_i32 = w_main.w_functype.w_restype == B.w_i32
+
+            if needs_argv:
+                self.tbh_includes.wl('#include "spy/list.h"')


here, instead of including spy/list.h, I'd just copy&paste the code for wrapping argv.
Something like this:

self.tbc.wb(""" #define spy_list_str spy__list$list__builtins$str$_ListImpl #define spy_list_str_new spy__list$list__builtins$str$_ListImpl$__new__ #define spy_list_str_push spy__list$list__builtins$str$_ListImpl$_push spy_list_str spy_wrap_argv(int argc, const char *argv[]) { ... } """)

it's not ideal, but it's just a very short amount of code and it saves lots of complications

Yeah,

At first I thought of this as well. Let me resolve it this way.

Okay, refactored it.

antocuni

there seem to be several leftovers from the previous attempt, but overall looks good 👍

antocuni · 2026-04-02T18:17:31Z

spy/backend/c/cmodwriter.py

-                }}
+
+            needs_argv = len(w_main.w_functype.params) == 1
+            returns_i32 = w_main.w_functype.w_restype == B.w_i32


why do you need this instead of calling vm.typecheck_main as it was before?

antocuni · 2026-04-02T18:18:02Z

spy/backend/c/cmodwriter.py

+            returns_i32 = w_main.w_functype.w_restype == B.w_i32
+
+            if needs_argv:
+                self.tbh.wb("""


I think this should go to the tbc, not tbh

antocuni · 2026-04-02T18:20:46Z

spy/backend/c/cmodwriter.py

+                            size_t size_str = strlen(argv[i]);
+                            spy_Str *allo = spy_str_alloc(size_str);


Suggested change

size_t size_str = strlen(argv[i]);

spy_Str *allo = spy_str_alloc(size_str);

size_t length = strlen(argv[i]);

spy_Str *s = spy_str_alloc(length);

I think it's better to call it length for consistency with the field in spy_Str.
Also, allo was a very werid name, better to use s (you need to fix it also below)

antocuni · 2026-04-02T18:21:33Z

spy/backend/c/cmodwriter.py

                """)
+
+            if needs_argv and returns_i32:
+                execution_code = f"""


Suggested change

execution_code = f"""

main_src = f"""

a bit of nitpick, but I think this is clearer. After all, it's the "source of the main function".

antocuni · 2026-04-02T18:22:11Z

spy/libspy/include/spy/list.h

@@ -0,0 +1,17 @@
+#ifndef SPY_LIST_H


I think this file is no longer needed, is it?

antocuni · 2026-04-02T18:22:25Z

spy/libspy/src/list.c

@@ -0,0 +1,20 @@
+#include "spy/list.h"


same as above, delete it

antocuni · 2026-04-02T18:30:00Z

spy/tests/test_cli.py

+        main_c = self.tmpdir.join("src", "return_exit_code_src.c")
+        assert main_c.exists()
+        csrc = main_c.read()
+        assert "return 99;" in csrc


I think I have already deleted/refactored these tests during the review of #419, see e.g. this commit :)
3ebdaa8

There are few code style problems:

there is no need for a "cwrite" test for every case. The existing test_cwrite checks that the option work, but we shouldn't test cwrite for every single combination of options.

Also, the rest of the file follows different naming convention, e.g. it always use src = """...""" for the source to compile. Please follow them for consistency.

antocuni · 2026-04-02T18:30:58Z

spy/tests/test_cli.py

+        csrc = main_c.read()
+        assert "return 99;" in csrc
+
+    def test_nested_functions_exit_code_cwrite(self):


this was also deleted by 3ebdaa8 . It seems to test that you can call a function, but that's already tested elsewhere.

Please look at that commit and refactor/redelete all the tests which I have already fixed :)

antocuni · 2026-04-02T18:31:55Z

spy/backend/c/cbackend.py


 import py.path

+import spy.libspy


I think this is not needed?

antocuni · 2026-04-02T18:36:22Z

@kanin-kearpimy also, I just merged #445 which (rightly) adds the executable name as the first argv.
Make sure to test the same behavior also for the compiled case, plase 🙏

kanin-kearpimy requested a review from antocuni April 2, 2026 13:13

kanin-kearpimy force-pushed the 353-add-support-for-argv-and-exit-code branch from cd8d6be to c83e239 Compare April 2, 2026 14:50

antocuni reviewed Apr 2, 2026

View reviewed changes

KANIN KEARPIMY added 4 commits April 3, 2026 00:23

implement argv param

6f1cfdf

move hardcoded spy_wrap_argv to list.c

43695cb

remove main arugment guard

fdc87e3

directly emit code

09f3c14

kanin-kearpimy force-pushed the 353-add-support-for-argv-and-exit-code branch from 542f549 to 09f3c14 Compare April 2, 2026 17:23

antocuni requested changes Apr 2, 2026

View reviewed changes

		size_t size_str = strlen(argv[i]);
		spy_Str *allo = spy_str_alloc(size_str);

Conversation

kanin-kearpimy commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

However, there is a tricky part happening

Uh oh!

antocuni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antocuni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antocuni commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kanin-kearpimy commented Apr 2, 2026 •

edited

Loading