[rcore] fix linkage warnings under windows

[RicoP]

Under windows I kept getting the linkage warnings

rcore.c(161): warning C4028: formal parameter 1 different from declaration
rcore.c(161): warning C4028: formal parameter 2 different from declaration
rcore.c(162): warning C4028: formal parameter 1 different from declaration
rcore.c(162): warning C4028: formal parameter 2 different from declaration
rcore.c(163): warning C4028: formal parameter 3 different from declaration
rcore.c(163): warning C4028: formal parameter 5 different from declaration
rcore.c(163): warning C4028: formal parameter 7 different from declaration
rcore.c(504): warning C4273: 'Sleep': inconsistent dll linkage
C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um\synchapi.h(684): note: see previous definition of 'Sleep'

because the windows functions weren't exactly defined as in windows.h.

[raysan5]

@RicoP This issue has not been reported by anyone in 11 years, why did it happen to you?

[RicoP]

Well I am experimenting with unity builds, maybe this changed something in the behavior of msvc.

[RicoP]

@raysan5 are you concerned my changes could break something?

[raysan5]

@RicoP Yes, it's a concern. I need to test it carefully.

[RicoP]

Fair enough

[RicoP]

@raysan5 I was thinking to make a separate header file, something like "rwin32.h" that contains all windows.h predefs. This could also speed up compilation on windows.

[orcmid]

@RicoP

@raysan5 I was thinking to make a separate header file, something like "rwin32.h" that contains all windows.h predefs. This could also speed up compilation on windows.

I don't understand this. It seems like it would have separation of raylib.h from windows.h backwards. The reliable approach is to have any need for windows.h facilities isolated in a module (with header file) that isolates those dependencies, via custom-to-your-app API, separate from the code that includes raylib.h.

What compiler are you using, and what platform are you compiling for? Since you seem to be using Visual Studio Code, are you also using the free Visual Studio Build Tools for the VC/C++ compiler?

PS: What does Unity have to do with it?

[RicoP]

@RicoP

@raysan5 I was thinking to make a separate header file, something like "rwin32.h" that contains all windows.h predefs. This could also speed up compilation on windows.

I don't understand this. It seems like it would have separation of raylib.h from windows.h backwards. The reliable approach is to have any need for windows.h facilities isolated in a module (with header file) that isolates those dependencies, separate from the code that includes raylib.h.

What compiler are you using, and what platform are you compiling for? Since you seem to be using Visual Studio Code, are you also using the free Visual Studio Build Tools for the VC/C++ compiler?

PS: What does Unity have to do with it?

Unity builds have nothing to do with the unity game engine.

And the predefines are there such that windows.h doesn't have to be included at all which a lot of libs do that raylib uses.

[orcmid]

@RicoP
And the predefines are there such that windows.h doesn't have to be included at all which a lot of libs do that raylib uses.

So how is there a disparity if windows.h (or any of the many Microsoft headers that windows.h sucks in) are not included in the compile of your code? Could this disparity be introduced by declarations in another of the rcore.c includes along with PLATFORM_DEEKSTOP_RGFW being defined in your build of rcore.c?

Also, what version of raylib are you using? The latest main of rcore.c has different line numbers for those complaints. Finally, have you touched raylib's config.h?

[RicoP]

@RicoP
And the predefines are there such that windows.h doesn't have to be included at all which a lot of libs do that raylib uses.

So how is there a disparity if windows.h (or any of the many Microsoft headers that windows.h sucks in) are not included in the compile of your code? Could this disparity be introduced by declarations in another of the rcore.c includes along with PLATFORM_DEEKSTOP_RGFW being defined in your build of rcore.c?

It comes probably from another c file including windows.h which causes the compiler to notice that the predefines are actually different.

Also, what version of raylib are you using? The latest main of rcore.c has different line numbers for those complaints. Finally, have you touched raylib's config.h?

I am currently using the stable 5.5 version but the lines in my diff are all the same

[orcmid]

@RicoP
It comes probably from another c file including windows.h which causes the compiler to notice that the predefines are actually different.

C/C++ compilers are not supposed to do that. A C++ linker could if there's a prototype mismatch, but that's not what is happening here.

The term "predefines" is being used a lot here. Aren't we talking about declarators of functions expected to be defined externally?

Those error messages suggest that there have already been conflicting declarators or even an implementing declaration in the compile of rcore.c (or something rcore.c is #included in). That's very weird.

[RicoP]

@RicoP
It comes probably from another c file including windows.h which causes the compiler to notice that the predefines are actually different.

C/C++ compilers are not supposed to do that.

They do in unity builds.

[sleeptightAnsiC]

@RicoP How do you reproduce this? With cmake's UNITY_BUILD property? Could you report an issue about this with minimal reproduction steps? Maintainers could then properly validate the PR and probably fix other similar issues. Notice that raylib uses variety of build systems and targets multiple environments.

---

It comes probably from another c file including windows.h which causes the compiler to notice that the predefines are actually different.

C/C++ compilers are not supposed to do that.

They do in unity builds.

Yes. In "unity" builds every header file and source file is inlined into one translation unit. This issue can manifest there.

---

raylib/src/rcore.c

Lines 164 to 173 in 11090ab

	#if (defined(_WIN32) && !defined(PLATFORM_DESKTOP_RGFW)) || (defined(_MSC_VER) && defined(PLATFORM_DESKTOP_RGFW))
	#ifndef MAX_PATH
	#define MAX_PATH 1025
	#endif
	__declspec(dllimport) unsigned long __stdcall GetModuleFileNameA(void *hModule, void *lpFilename, unsigned long nSize);
	__declspec(dllimport) unsigned long __stdcall GetModuleFileNameW(void *hModule, void *lpFilename, unsigned long nSize);
	__declspec(dllimport) int __stdcall WideCharToMultiByte(unsigned int cp, unsigned long flags, void *widestr, int cchwide, void *str, int cbmb, void *defchar, int *used_default);
	__declspec(dllimport) unsigned int __stdcall timeBeginPeriod(unsigned int uPeriod);
	__declspec(dllimport) unsigned int __stdcall timeEndPeriod(unsigned int uPeriod);
	#elif defined(__linux__)

I wonder why rcore.c is redefining those instead of including proper Windows headers (<timeapi.h>, <libloaderapi.h> and <stringapiset.h>, OR just <windows.h>) ? I guess it's maybe because Windows headers pollute the unit and make compilation super slow, but still, current approach is fragile. It may break between different MSVC versions and for compilers that try to emulate MSVC (e.g. clang-cl). There should be at least a note in rcore.c mentioning why it was done this way.

[RicoP]

@RicoP How do you reproduce this? With cmake's UNITY_BUILD property? Could you report an issue about this with minimal reproduction steps? Maintainers could then properly validate the PR and probably fix other similar issues. Notice that raylib uses variety of build systems and targets multiple environments.

I haven't looked into the UNITY_BUILD option of cmake yet, because I don't use cmake. I basically made my own unity build using this very hacky c file that includes all other c files. The code is very messy and work in progress.

https://github.com/RicoP/unityraylib/blob/main/src/unityraylib.c

[orcmid]

@RicoP @sleeptightAnsiC

FIRST TEST: Do a "Unity" compile of only rcore.c to produce an rcore.obj and see if that does not exhibit the problem.

By design, raylib depends on the raylib components having been individually compiled, whether for subsequent inclusion in a static *.lib or directly used in a linker stage. This is rather ... umm ... traditional and expected.

If that is the problem @RicoP is experiencing, I don't think it is raylib's to fix.

[RicoP]

@RicoP @sleeptightAnsiC

FIRST TEST: Do a "Unity" compile of only rcore.c to produce an rcore.obj and see if that does not exhibit the problem.

By design, raylib depends on the raylib components having been individually compiled, whether for subsequent inclusion in a static *.lib or directly used in a linker stage. This is rather ... umm ... traditional and expected.

If that is the problem @RicoP is experiencing, I don't think it is raylib's to fix.

Unity builds are a common technique that speed up builds and reduce binary sizes in complex projects. Even when raylib doesn't support unity builds officially the windows predefs are still technically "wrong" even when it's rather unlikely they going to cause issues. So I think my PR is still worth merging.

[orcmid]

@Roc0P

I wonder why rcore.c is redefining those instead of including proper Windows headers (<timeapi.h>, <libloaderapi.h> and <stringapiset.h>, OR just <windows.h>) ? I guess it's maybe because windows.h pollutes the unit and makes compilation super slow, but still, current approach is fragile. It may break between different MSVC versions and for compilers that try to emulate MSVC (e.g. clang-cl). There should be at least a note in rcore.c mentioning why it was done this way.

The problem of using windows.h is that there are conflicts with some names of raylib API functions. (The inspiration for raylib predates windows.h :). Taking apart windows.h into the components that are the only ones needed is discouraged by MSFT and I stopped doing that years ago.

There is a safer way to accomplish what you suggest that will not have edge cases or fragility with regard to unlikely changes to the Win32 APIs and libraries. It does mean adding another *.c and perhaps *.h to the set of raylib components and it is unclear whether that build-procedure breaking-change is worthwhile. Perhaps for 6.0.

MSVC's and Clang's handling of C/C++ are not an issue. They will never change the compile > object > link > .exe model and their degrees of ISO compliance. The same goes for gcc. "Unity" is the outlier here.

[orcmid]

@RicoP @sleeptightAnsiC
FIRST TEST: Do a "Unity" compile of only rcore.c to produce an rcore.obj and see if that does not exhibit the problem.

Unity builds are a common technique that speed up builds and reduce binary sizes in complex projects. Even when raylib doesn't support unity builds officially the windows predefs are still technically "wrong" even when it's rather unlikely they going to cause issues. So I think my PR is still worth merging.

Are you confirming that this is the problem?

I think you are ignoring the advances in other compiler systems, including MSVC, for generating optimizations of code at link time without damaging the fundamental model.

In any case, I claim there are better ways to isolate Win32 function dependencies that guards against collisions with like-named raylib functions, working for all build schemes targeting Windows (even with the "Unity" approach). It would even survive unlikely future additions to Win32 that collide with the raylib APIs.

[RicoP]

@RicoP @sleeptightAnsiC
FIRST TEST: Do a "Unity" compile of only rcore.c to produce an rcore.obj and see if that does not exhibit the problem.

Unity builds are a common technique that speed up builds and reduce binary sizes in complex projects. Even when raylib doesn't support unity builds officially the windows predefs are still technically "wrong" even when it's rather unlikely they going to cause issues. So I think my PR is still worth merging.

Are you confirming that this is the problem?

I think you are ignoring the advances in other compiler systems, including MSVC, for generating optimizations of code at link time without damaging the fundamental model.

In any case, I claim there are better ways to isolate Win32 function dependencies that guards against collisions with like-named raylib functions, working for all build schemes targeting Windows (even with the "Unity" approach). It would even survive unlikely future additions to Win32 that collide with the raylib APIs.

That isn't the problem thou 😅

[sleeptightAnsiC]

The problem of using windows.h is that there are conflicts with some names of raylib API functions.

Okey, I see. <windows.h> is out of the question.

I tried to fix it by including other Windows headers directly but those throw error due to lack of predefines from... ehh... from <windows.h>... (great job Microsoft, everything is dependent on setup from windows.h which causes no issues at all)

I'm afraid, this is super risky to fix, if this only produces the warnings. Redefining function declarations is not good either. Raylib names clashing with Win32 is really just a technical debt and rcore.c doing all those gymnastics around it may backfire someday. [edit] related to #1217

However, adding __declspec(dllimport) before void __stdcall Sleep(unsigned long msTimeout); is probably desired and should be merged, since other Win32 calls already use it.

---

@RicoP Out of curiosity, does the patch below fix your issue? It wasn't causing any name clashing on my side but it's really hard to tell (I'm using MSVC tool-chain on Linux, emulated under Wine, had no time to test it on Win64).

[NOT A VALID FIX, just experimenting]

diff --git a/src/rcore.c b/src/rcore.c
index 165c6f1..a65760d 100644
--- a/src/rcore.c
+++ b/src/rcore.c
@@ -162,14 +162,11 @@
 
 // Platform specific defines to handle GetApplicationDirectory()
 #if (defined(_WIN32) && !defined(PLATFORM_DESKTOP_RGFW)) || (defined(_MSC_VER) && defined(PLATFORM_DESKTOP_RGFW))
-    #ifndef MAX_PATH
-        #define MAX_PATH 1025
-    #endif
-__declspec(dllimport) unsigned long __stdcall GetModuleFileNameA(void *hModule, void *lpFilename, unsigned long nSize);
-__declspec(dllimport) unsigned long __stdcall GetModuleFileNameW(void *hModule, void *lpFilename, unsigned long nSize);
-__declspec(dllimport) int __stdcall WideCharToMultiByte(unsigned int cp, unsigned long flags, void *widestr, int cchwide, void *str, int cbmb, void *defchar, int *used_default);
-__declspec(dllimport) unsigned int __stdcall timeBeginPeriod(unsigned int uPeriod);
-__declspec(dllimport) unsigned int __stdcall timeEndPeriod(unsigned int uPeriod);
+    #define _AMD64_ // <---  FIXME: this would need to be detected, not hard-coded... (windows.h sets this)
+    #include <stringapiset.h>  // WideCharToMultiByte()
+    #include <libloaderapi.h>  // GetModuleFileNameA() GetModuleFileNameW()
+    #include <timeapi.h>       // timeBeginPeriod() timeEndPeriod()
+    #include <synchapi.h>      // Sleep() (required for: WaitTime())
 #elif defined(__linux__)
     #include <unistd.h>
 #elif defined(__FreeBSD__)
@@ -509,11 +506,6 @@ static void ScanDirectoryFilesRecursively(const char *basePath, FilePathList *li
 static void RecordAutomationEvent(void); // Record frame events (to internal events array)
 #endif
 
-#if defined(_WIN32) && !defined(PLATFORM_DESKTOP_RGFW)
-// NOTE: We declare Sleep() function symbol to avoid including windows.h (kernel32.lib linkage required)
-void __stdcall Sleep(unsigned long msTimeout);              // Required for: WaitTime()
-#endif
-
 #if !defined(SUPPORT_MODULE_RTEXT)
 const char *TextFormat(const char *text, ...);              // Formatting of text with variables to 'embed'
 #endif // !SUPPORT_MODULE_RTEXT

[RicoP]

---

@RicoP Out of curiosity, does the patch below fix your issue? It wasn't causing any name clashing on my side but it's really hard to tell (I'm using MSVC tool-chain on Linux, emulated under Wine, had no time to test it on Win64).

[NOT A VALID FIX, just experimenting]

diff --git a/src/rcore.c b/src/rcore.c
index 165c6f1..a65760d 100644
--- a/src/rcore.c
+++ b/src/rcore.c
@@ -162,14 +162,11 @@
 
 // Platform specific defines to handle GetApplicationDirectory()
 #if (defined(_WIN32) && !defined(PLATFORM_DESKTOP_RGFW)) || (defined(_MSC_VER) && defined(PLATFORM_DESKTOP_RGFW))
-    #ifndef MAX_PATH
-        #define MAX_PATH 1025
-    #endif
-__declspec(dllimport) unsigned long __stdcall GetModuleFileNameA(void *hModule, void *lpFilename, unsigned long nSize);
-__declspec(dllimport) unsigned long __stdcall GetModuleFileNameW(void *hModule, void *lpFilename, unsigned long nSize);
-__declspec(dllimport) int __stdcall WideCharToMultiByte(unsigned int cp, unsigned long flags, void *widestr, int cchwide, void *str, int cbmb, void *defchar, int *used_default);
-__declspec(dllimport) unsigned int __stdcall timeBeginPeriod(unsigned int uPeriod);
-__declspec(dllimport) unsigned int __stdcall timeEndPeriod(unsigned int uPeriod);
+    #define _AMD64_ // <---  FIXME: this would need to be detected, not hard-coded... (windows.h sets this)
+    #include <stringapiset.h>  // WideCharToMultiByte()
+    #include <libloaderapi.h>  // GetModuleFileNameA() GetModuleFileNameW()
+    #include <timeapi.h>       // timeBeginPeriod() timeEndPeriod()
+    #include <synchapi.h>      // Sleep() (required for: WaitTime())
 #elif defined(__linux__)
     #include <unistd.h>
 #elif defined(__FreeBSD__)
@@ -509,11 +506,6 @@ static void ScanDirectoryFilesRecursively(const char *basePath, FilePathList *li
 static void RecordAutomationEvent(void); // Record frame events (to internal events array)
 #endif
 
-#if defined(_WIN32) && !defined(PLATFORM_DESKTOP_RGFW)
-// NOTE: We declare Sleep() function symbol to avoid including windows.h (kernel32.lib linkage required)
-void __stdcall Sleep(unsigned long msTimeout);              // Required for: WaitTime()
-#endif
-
 #if !defined(SUPPORT_MODULE_RTEXT)
 const char *TextFormat(const char *text, ...);              // Formatting of text with variables to 'embed'
 #endif // !SUPPORT_MODULE_RTEXT

Thanks, I gonna give it a try when I finished my work.

[RicoP]

@RicoP Out of curiosity, does the patch below fix your issue? It wasn't causing any name clashing on my side but it's really hard to tell (I'm using MSVC tool-chain on Linux, emulated under Wine, had no time to test it on Win64).

[NOT A VALID FIX, just experimenting]

diff --git a/src/rcore.c b/src/rcore.c
index 165c6f1..a65760d 100644
--- a/src/rcore.c
+++ b/src/rcore.c
@@ -162,14 +162,11 @@
 
 // Platform specific defines to handle GetApplicationDirectory()
 #if (defined(_WIN32) && !defined(PLATFORM_DESKTOP_RGFW)) || (defined(_MSC_VER) && defined(PLATFORM_DESKTOP_RGFW))
-    #ifndef MAX_PATH
-        #define MAX_PATH 1025
-    #endif
-__declspec(dllimport) unsigned long __stdcall GetModuleFileNameA(void *hModule, void *lpFilename, unsigned long nSize);
-__declspec(dllimport) unsigned long __stdcall GetModuleFileNameW(void *hModule, void *lpFilename, unsigned long nSize);
-__declspec(dllimport) int __stdcall WideCharToMultiByte(unsigned int cp, unsigned long flags, void *widestr, int cchwide, void *str, int cbmb, void *defchar, int *used_default);
-__declspec(dllimport) unsigned int __stdcall timeBeginPeriod(unsigned int uPeriod);
-__declspec(dllimport) unsigned int __stdcall timeEndPeriod(unsigned int uPeriod);
+    #define _AMD64_ // <---  FIXME: this would need to be detected, not hard-coded... (windows.h sets this)
+    #include <stringapiset.h>  // WideCharToMultiByte()
+    #include <libloaderapi.h>  // GetModuleFileNameA() GetModuleFileNameW()
+    #include <timeapi.h>       // timeBeginPeriod() timeEndPeriod()
+    #include <synchapi.h>      // Sleep() (required for: WaitTime())
 #elif defined(__linux__)
     #include <unistd.h>
 #elif defined(__FreeBSD__)
@@ -509,11 +506,6 @@ static void ScanDirectoryFilesRecursively(const char *basePath, FilePathList *li
 static void RecordAutomationEvent(void); // Record frame events (to internal events array)
 #endif
 
-#if defined(_WIN32) && !defined(PLATFORM_DESKTOP_RGFW)
-// NOTE: We declare Sleep() function symbol to avoid including windows.h (kernel32.lib linkage required)
-void __stdcall Sleep(unsigned long msTimeout);              // Required for: WaitTime()
-#endif
-
 #if !defined(SUPPORT_MODULE_RTEXT)
 const char *TextFormat(const char *text, ...);              // Formatting of text with variables to 'embed'
 #endif // !SUPPORT_MODULE_RTEXT

I tested your patch and on my x64 windows machine this works just as well. I tested a regular build and my unity build.

I do wonder thou if the _AMD64_ define only works on 64 bit machines.

[sleeptightAnsiC]

I do wonder thou if the _AMD64_ define only works on 64 bit machines.

This define is not very portable outside of x86_64 (amd64/x64) as it may change how those headers work internally. Usually <windows.h> sets this macro, and without it, including the header throws an error.

We can maybe create a very simplified detection on Raylib side and detect only those Windows platforms supported by raylib (there are only 4, if I remember right: x86, x86_64, arm, arm64) but this require some testing across MSVC tool-chains and with older versions of Windows.

This is how `window.h` detects all platforms in MSVC 19.40.33811

#if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_AMD64_) && !defined(_ARM_) && !defined(_ARM64_) && !defined(_ARM64EC_) && defined(_M_IX86)
#define _X86_
#if !defined(_CHPE_X86_ARM64_) && defined(_M_HYBRID)
#define _CHPE_X86_ARM64_
#endif
#endif

#if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_AMD64_) && !defined(_ARM_) && !defined(_ARM64_) && (defined(_M_AMD64) || defined(_M_ARM64EC))
#define _AMD64_
#endif

#if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_AMD64_) && !defined(_ARM_) && !defined(_ARM64_) && !defined(_ARM64EC_) && defined(_M_ARM)
#define _ARM_
#endif

#if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_AMD64_) && !defined(_ARM_) && !defined(_ARM64_) && !defined(_ARM64EC_) && defined(_M_ARM64)
#define _ARM64_
#endif

#if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_ARM_) && !defined(_ARM64_) && !defined(_ARM64EC_) && defined(_M_ARM64EC)
#define _ARM64EC_
#endif

#if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_AMD64_) && !defined(_ARM_) && !defined(_ARM64_) && !defined(_ARM64EC_) && defined(_M_M68K)
#define _68K_
#endif

#if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_IA64_) && !defined(_AMD64_) && !defined(_ARM_) && !defined(_ARM64_) && !defined(_ARM64EC_) && defined(_M_MPPC)
#define _MPPC_
#endif

#if !defined(_68K_) && !defined(_MPPC_) && !defined(_X86_) && !defined(_M_IX86) && !defined(_AMD64_) && !defined(_ARM_) && !defined(_ARM64_) && !defined(_ARM64EC_) && defined(_M_IA64)
#if !defined(_IA64_)
#define _IA64_
#endif /* !_IA64_ */
#endif

#ifndef _MAC
#if defined(_68K_) || defined(_MPPC_)
#define _MAC
#endif
#endif