Mindwerks / wildmidi

WildMIDI is a simple software midi player which has a core softsynth library that can be used with other applications.
https://github.com/Mindwerks/wildmidi
Other
201 stars 41 forks source link

Silence warnings when compiling as ANSI C #243

Closed Clownacy closed 6 months ago

Clownacy commented 1 year ago

Compiling WildMIDI as ANSI C with GCC and with warnings enabled causes many warning messages to be printed which complain about __FUNCTION__ not existing in C89. This commit works around the issue by only using it when compiling as C99 or newer. In C89, the function string is simply dummied-out instead.

While I was at it, I used some macro magic to hide most uses of __FUNCTION__ and __LINE__.

psi29a commented 6 months ago

Wow, thanks @Clownacy , it's really appreciated. I liked reading your blog ;)

sezero commented 6 months ago

This unnecessarily cripples debug messages when not building in c99 mode

sezero commented 6 months ago

This unnecessarily cripples debug messages when not building in c99 mode

No, I am not happy with this. @Clownacy , @psi29a :

As I understand it, new gcc versions emit -Wpedantic for __FUNCTION__ (and even for __func__ if you try hard enough.)

Is the following good ?

diff --git a/include/wm_error.h b/include/wm_error.h
index 85514cb..1e1e307 100644
--- a/include/wm_error.h
+++ b/include/wm_error.h
@@ -52,6 +52,20 @@ extern int _WM_Global_ErrorI;

 #if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L) || (defined(__cplusplus) && __cplusplus >= 201103L)
 #define _WM_FUNCTION __func__
+#elif defined(__clang__)
+#define _WM_FUNCTION __func__
+#pragma GCC diagnostic ignored "-Wpedantic"
+#elif defined(__GNUC__) && (__GNUC__ > 2)
+#if (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6))
+#pragma GCC diagnostic ignored "-Wpedantic"
+#endif
+#define _WM_FUNCTION __func__
+#elif defined(__GNUC__)
+#define _WM_FUNCTION __FUNCTION__
+#elif defined(__WATCOMC__)
+#define _WM_FUNCTION __FUNCTION__
+#elif defined(_MSC_VER) && (_MSC_VER >= 1300) && defined(__FUNCTION__) /* VC7++ */
+#define _WM_FUNCTION __FUNCTION__
 #else
 #define _WM_FUNCTION "<unknown>"
 #endif
Clownacy commented 6 months ago

Boost has a similar series of checks, though it needs to be wrapped in a dummy function in order to work properly, since __FUNCSIG__ is only defined within a function. That has me wondering if a similar limitation applies to checking if __FUNCTION__ is defined.

Disabling -Wpedantic is unfortunate, since it could hide other code flaws, but if GCC insists on emitting a warning no matter what, then I suppose nothing else can be done. The only alternative I can think of is foregoing _WM_FUNCTION completely and manually inserting the function name into each call to _WM_GLOBAL_ERROR.

Otherwise, those new checks look pretty good.

sezero commented 6 months ago

Boost has a similar series of checks, though it needs to be wrapped in a dummy function in order to work properly, since __FUNCSIG__ is only defined within a function. That has me wondering if a similar limitation applies to checking if __FUNCTION__ is defined.

Disabling -Wpedantic is unfortunate, since it could hide other code flaws, but if GCC insists on emitting a warning no matter what, then I suppose nothing else can be done. The only alternative I can think of is foregoing _WM_FUNCTION completely and manually inserting the function name into each call to _WM_GLOBAL_ERROR.

Otherwise, those new checks look pretty good.

OK, thanks. I am tempted to add this with a few additions of INTEL_COMPILER and ICC cases

@psi29a ?

sezero commented 6 months ago

OK, thanks. I am tempted to add this with a few additions of INTEL_COMPILER and ICC cases

@psi29a ?

OK, I tweaked gcc version detection for -Wpedantic disablement and kept the rest as is, i.e. only the compilers I'm sure of, so as not to add any bugs. The final version of my patch is below, to be applied if no objections until Apr. 10.

wm_error: Allow the use of __FUNCTION__ or __func__ when available,

.. and disable -Wpedantic for gcc >= 4.6 and clang. This uncripples
debug messages when not building in c99 mode since commit 473103367
Reference issue: https://github.com/Mindwerks/wildmidi/pull/243

diff --git a/include/wm_error.h b/include/wm_error.h
index 85514cb..f163265 100644
--- a/include/wm_error.h
+++ b/include/wm_error.h
@@ -50,8 +50,23 @@ enum {
 extern char * _WM_Global_ErrorS;
 extern int _WM_Global_ErrorI;

+/* TODO: add more here, as needed. */
 #if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L) || (defined(__cplusplus) && __cplusplus >= 201103L)
 #define _WM_FUNCTION __func__
+#elif defined(__clang__)
+#define _WM_FUNCTION __func__
+#pragma GCC diagnostic ignored "-Wpedantic"
+#elif defined(__GNUC__) && (__GNUC__ >= 3)
+#define _WM_FUNCTION __func__
+#if (__GNUC__ + (__GNUC_MINOR__ >= 6) > 4)  /* gcc >= 4.6 */
+#pragma GCC diagnostic ignored "-Wpedantic"
+#endif
+#elif defined(__GNUC__)
+#define _WM_FUNCTION __FUNCTION__
+#elif defined(__WATCOMC__)
+#define _WM_FUNCTION __FUNCTION__
+#elif defined(_MSC_VER) && (_MSC_VER >= 1300) && defined(__FUNCTION__) /* VC7++ */
+#define _WM_FUNCTION __FUNCTION__
 #else
 #define _WM_FUNCTION "<unknown>"
 #endif
psi29a commented 6 months ago

I'm fine with this or a revert and rework.

The Todo pretty much covers other use-cases if/when we get to them.

sezero commented 6 months ago

OK, patch applied as https://github.com/Mindwerks/wildmidi/commit/77edece9ce4a6f8d6c9d5922598828c84e86475b