Skip to content

markused: fix for thread waiter usage #24580

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

Merged
merged 1 commit into from
May 29, 2025

Conversation

felipensp
Copy link
Member

Fix #24577

Copy link

Connected to Huly®: V_0.6-22945

@felipensp felipensp marked this pull request as ready for review May 26, 2025 12:02
@ctkjose
Copy link
Contributor

ctkjose commented May 26, 2025

The go statement is a coroutine. In the issue #24577 the go statement became a spawn because the option -use-coroutines was not used.

The problem is that spawn does not use v code that the builder could use to detect dependencies.

If modifying markused.v you could just do:

if table.gostmts > 0 {
	all_fn_root_names << 'free'
}

Using table.gostmts should be an acceptable way to assume a spawn without the need to modify the others.

PS In vlib/v/gen/c/cheaders.v we declare void v_free(voidptr ptr);. This is hardwired, so was it meant to be a base function always included? It was committed in 88810a7 for -gc boehm which added the C.GC_* functions. Wonder @spytheman could give us more clues.

@spytheman
Copy link
Member

spytheman commented May 29, 2025

This is hardwired, so was it meant to be a base function always included?

markused evolved separately from the GC integration.
It was simpler (and buggier, measured by the % of .v files in the repository that could not be compiled with -skip-unused, or that could, at the price of still having a lot of unused code generated) initially, using a much larger set of hardcoded APIs.

Ideally, it should keep track of all of the used functionality, that the backends would then generate calls for, including free/malloc, so that compiling a .v file with the C backend, containing just pub fn f() {} should produce a simple C function, with no additional code.

@spytheman
Copy link
Member

I agree, that using if table.gostmts > 0 { would be simpler, but it will also have a lower precision. Consider a go f() call, with fn f() { println('hi') } for example, that does not return anything, and that will not generate a _v_free() call in the V gowrapper function.

Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work.

@spytheman spytheman merged commit eeeef41 into vlang:master May 29, 2025
76 of 77 checks passed
@ctkjose
Copy link
Contributor

ctkjose commented May 29, 2025

I agree, that using if table.gostmts > 0 { would be simpler, but it will also have a lower precision. Consider a go f() call, with fn f() { println('hi') } for example, that does not return anything, and that will not generate a _v_free() call in the V gowrapper function.

If I recall correctly the fix in this PR will also inject free regardless, it also adds more noise to the table.

The issue is that the spawn_and_go.v does not use actual v code for it to properly detect dependencies. Maybe an alternate version of parser.register_auto_import() that allows to register functions would avoid having to hack situations like this.

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

Successfully merging this pull request may close these issues.

Spawning anonymous functions that have no input arguments (example from book: Getting Started with V Programming) fails
3 participants