Declare tic80_tick's callbacks as they are defined and called - #2981
Open
annejan wants to merge 1 commit into
Open
Declare tic80_tick's callbacks as they are defined and called#2981annejan wants to merge 1 commit into
annejan wants to merge 1 commit into
Conversation
include/tic80.h declares the counter and freq callbacks as u64 (*)(),
while tic.c defines tic80_tick taking CounterCallback and FreqCallback,
which are u64 (*)(void*), and core.c calls them with an argument:
core->data->counter(core->data->data)
Under C17 and earlier an empty parameter list means "unspecified", so the
mismatch is hidden. C23 gives () the same meaning as (void), and the
declaration and definition become incompatible types, so building with a
C23 compiler fails:
error: conflicting types for 'tic80_tick'
The studio already implements these correctly, in run.c, taking void* and
receiving .data. The two standalone callers did not, and worked only
because the callee ignored the extra argument.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
|
Looks good at a glance, though I'm not sure I understand technically which approach might be best. If this is fixing the minority case that seems reasonable to go with the majority unless anyone else kows better. |
Author
|
I bumped into the issue porting TIC-80 to our own fantasy console of sorts https://nicolaielectronics.nl/tanmatsu/ In my opinion it's a matter of future proofing. |
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.
include/tic80.hdeclares the timing callbacks asu64 (*)(), whiletic.cdefinestic80_ticktakingCounterCallbackandFreqCallback, which areu64 (*)(void*), andcore.ccalls them with an argument:Under C17 and earlier an empty parameter list means "unspecified arguments", so the mismatch is hidden. C23 gives
()the same meaning as(void), and the declaration and definition become incompatible types. Building with a C23 compiler fails:GCC 15 defaults to
-std=gnu23, so this is reached simply by building with a current toolchain. I hit it porting TIC-80 to the ESP32-P4, where the SDK compiles at C23; the workaround there is pinning-std=gnu17, which seemed worth replacing with a real fix.The change
run.calready implements these callbacks correctly, takingvoid*and receiving.data. Only the two standalone callers did not, and they worked because the callee ignored the extra argument:include/tic80.h— declare the parameters the definition actually hassrc/system/sdl/player.c— its two static callbacks takevoid*src/system/libretro/tic80_libretro.c— likewiseFive lines. No behaviour change; the argument was already being passed at every call.
Tested by building the SDL target with GCC 15.
Happy to take the opposite approach instead if you would rather the callbacks genuinely take no argument — that would mean changing the typedefs in
api.h, the four call sites incore.c, andrun.c, and dropping the.dataplumbing. I went with the smaller change that keeps the existing design.