Skip to content

Conversation

@qmuntal
Copy link
Contributor

@qmuntal qmuntal commented Dec 17, 2025

What issue is this addressing?

Updates #343

What type of issue is this addressing?

Refactor

What this PR does | solves

setg_trampoline and call5 function signatures don't need to live in autogenerated files, as they don't depend on any input from gen.go.

This will make future enchancements to autogenerated code a bit easier.

@qmuntal qmuntal changed the title move setg and call5 out of autogenerated files fakecgo: move setg and call5 out of autogenerated files Dec 17, 2025
Copy link
Member

@hajimehoshi hajimehoshi left a comment

Choose a reason for hiding this comment

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

@TotallyGamerJet
Copy link
Collaborator

Hmm I'm not sure how I feel about it. The way fakecgo is written is to mimick the structure of runtime/cgo. go_utils is gcc_utils.c in the runtime so that changes can easily be kept in sync. Since these functions aren't found in the original code I put them in the generated file. I'm not fully opposed to it if we don't care to be exactly the same as the runtime.

@qmuntal
Copy link
Contributor Author

qmuntal commented Dec 17, 2025

I can put them in a separate file which would contain functions that don't appear upstream, if you wish.

@TotallyGamerJet
Copy link
Collaborator

I don't necessarily like the idea of adding another file to hold just two functions. What do you suggest it be called?

@qmuntal
Copy link
Contributor Author

qmuntal commented Dec 17, 2025

fakecgo.go. To solve #343 I'll have to add some more types and methods that are not in runtime/cgo, so that file is expected to grow a bit.

@qmuntal
Copy link
Contributor Author

qmuntal commented Dec 17, 2025

Done!

Copy link
Collaborator

@TotallyGamerJet TotallyGamerJet left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@hajimehoshi hajimehoshi merged commit 490ca62 into ebitengine:main Dec 17, 2025
41 checks passed
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.

3 participants