[MSWin] win32/GNUmakefile CFLAGS needs "-Wno-error=implicit-function-declaration" if building perl with gcc-14.1.0

[sisyphus]

Description

The mingw-w64 ports of gcc-14.1.0 currently available from https://winlibs.com turn "implicit declaration of function..." warnings into fatal errors.
This prevents ext/POSIX from building on all configurations.
It also stops -Duselongdouble builds at a very early stage because of an implicit function declaration (of isnanl) in sv.c.

Both of those failings can be addressed in win32/GNUmakefile by replacing:

EXTRACFLAGS	+= -std=c99
with
EXTRACFLAGS	+= -std=c99 -Wno-error=implicit-function-declaration

I'll make a PR for that in the next devel cycle, unless there's a better idea.

[haarg]

It seems like we should be fixing the use of implicit function declarations, not trying to ignore it.

[Leont]

Yeah, we had a similar issue before on Mac and ignoring these errors gave problems later on.

[sisyphus]

It seems like we should be fixing the use of implicit function declarations, not trying to ignore it.

The "isnanl" warning has been around ever since -Duselongdouble builds were enabled for Windows (about 10 years ago), and we've been ignoring it for all that time.
However, it's a pretty simple fix:

$ diff -u perl.h_orig perl.h
--- perl.h_orig 2024-04-27 19:35:14.000000000 +1000
+++ perl.h      2024-05-14 12:17:33.824537500 +1000
@@ -2676,6 +2676,7 @@
 #       endif
 #   endif
 #   ifndef Perl_isnan
+#       include <math.h>
 #       if defined(HAS_ISNANL) && !(defined(isnan) && defined(HAS_C99))
 #           define Perl_isnan(x) isnanl(x)
 #       elif defined(__sgi) && defined(__c99)  /* XXX Configure test needed */

Does that inclusion of math.h needs to be made conditional on something ? If so, conditional on what ?

The POSIX warnings have also been around for a long time:

POSIX.xs: In function 'fix_win32_tzenv':
POSIX.xs:1727:13: warning: implicit declaration of function 'putenv'; did you mean '_putenv'? [-Wimplicit-function-declaration]
 1727 |             putenv(newenv);

That one goes away if the offending occurrence of putenv is replaced by _putenv, as suggested.

But the warnings given in relation to the bessel functions such as the following one (and other similar ones) has me stumped for the moment:

POSIX.xs:604:23: warning: implicit declaration of function 'j0' [-Wimplicit-function-declaration]
  604 | #    define bessel_j0 j0
      |                       ^~
POSIX.xs:2198:22: note: in expansion of macro 'bessel_j0'
 2198 |             RETVAL = bessel_j0(x);

I haven't yet discovered what needs to be done in order to silence that one.
Hopefully, the penny will eventually drop.

[sisyphus]

The [-Wimplicit-function-declaration] warnings in relation to the bessel functions don't appear when building perl-5.38.0.
It's the recent addition of -std=c99 to ccflags that's causing them to appear.
Using perl-5.38.0, the following Inline::C script emits no compilation warnings:

use strict;
use warnings;
use Inline C => Config =>
  BUILD_NOISY => 1,
  FORCE_BUILD => 1,
#  CCFLAGSEX => '-std=c99', # Adds '-std=c99' to ccflags
  USING => 'ParseRegExp',
;
use Inline C => <<'EOC';

#define bessel_j0 j0

void foo(void) {
  printf("%.17g\n", bessel_j0(11));
}

EOC

foo();

But if I add -std=c99 to the ccflags by uncommenting in the "CCFLAGSEX" line then (still running perl-5.38.0) I see the following compilation warnings:

try_pl_0649.xs: In function 'foo':
try_pl_0649.xs:6:19: warning: implicit declaration of function 'j0' [-Wimplicit-function-declaration]
    6 | #define bessel_j0 j0
      |                   ^~
try_pl_0649.xs:9:21: note: in expansion of macro 'bessel_j0'
    9 |   printf("%.17g\n", bessel_j0(11));
      |

Any thoughts on how best to suppress those warnings without removing -std=c99 from the ccflags ?

UPDATE: I've just noticed that, with the -std=c99 flag set, the output of that script is rubbish.
I'll have to look more closely at what's going on tomorrow.

[tonycoz]

I suspect we should just be using isnan(...) for double/long double builds, in C99 this is a generic function that accepts any floating point type (though probably not __float128).

You're getting rubbish from j0() because without the prototype the 11 is passed as an int instead of as a double.

The putenv() code isn't actually called to my knowledge, switching win32 from emulating putenv to emulating setenv is on my tuit list (complicated by implicit sys).

Is your gcc UCRT or MSVCRT?

[sisyphus]

I suspect we should just be using isnan(...) for double/long double builds ...

Is there a good reason to avoid using isnanl on the long double builds ?

You're getting rubbish from j0() because without the prototype the 11 is passed as an int instead of as a double.

Is there a fix for that ?
UPDATE: Duh - subsequent experimentation shows me that providing the prototype fixes that, and also avoids the implicit-function-declaration warnings.
It would need to be a fix that also prevents the implicit-function-declaration warnings if we're disallowing use of -Wno-error=implicit-function-declaration.
The "quadmath" build is fine - no rubbish results and no warnings, even when -std=c99 is specified.
It's just the "double" and "long double" builds that are posing those problems.

My gcc is UCRT (with MCF threads),
Strawberry Perl recently provided a build of perl-5.39.10 built using gcc version 13.1.0 (MinGW-W64 x86_64-msvcrt-posix-seh, built by Brecht Sanders):
https://github.com/StrawberryPerl/Perl-Dist-Strawberry/releases/download/SP_5.39.10/strawberry-perl-5.39.10.1-64bit-portable.zip
It exhibits the same behaviour wrt the bessel functions.

[eli-schwartz]

You're getting rubbish from j0() because without the prototype the 11 is passed as an int instead of as a double.

Is there a fix for that ? It would need to be a fix that also prevents the implicit-function-declaration warnings if we're disallowing use of -Wno-error=implicit-function-declaration. The "quadmath" build is fine - no rubbish results and no warnings, even when -std=c99 is specified. It's just the "double" and "long double" builds that are posing those problems.

The fix is writing code in the C programming language. Implicit function declarations are not, as it turns out, the C programming language.

Well. It's valid K&R C. But in c99 this was made illegal and declared invalid, without a deprecation period. A notable move for a language that doesn't even like to deprecate things! To go right to deleting it!

The problem is, as observed, that you're using a function that has a return type (in a header file!) and asking the compiler to generate code without the header file to tell it what code to generate. In K&R C, it was allowable to say "I know from outside context that it returns an int, and I solemnly swear on my mother's grave that I am willing to have demons fly out of my nostrils if it turns out I was mistaken. Please don't warn me that I might have made a judgment error".

The correct solution is invariably to simply include the headers that you make use of. There are no downsides to including the headers that you make use of. Sometimes the answer is to include your own function prototypes, though.

GCC 14 and clang 16 finally caught up to the standard 25 years after the fact: rather than live in fear of C programmers saying "a compiler update made my code stop building, it must be a bug in the compiler" and simply allowing invalid C code, it is now a default error to try to compile invalid C code. This is a desperately needed fix, because it was always Undefined Behavior (even on GCC 13) which means even if the function actually was an int function the compiler is still allowed to do nasal demons, and compilers are increasingly good at optimizing your code by assuming UB can't exist through a variety of clever mechanisms including making blocks of code a no-op and generating zero machine code for parts of your program.

This is not cosmetic, not even for GCC 13. It is buggy and prone to both crashing and succeeding-but-with-incorrect-answers, and you cannot predict when it will do either one, but the chances go way up when building with optimizations of any sort, or when compiling for platforms other than i686/amd64, or when upgrading to new compilers.

The compiler is telling you the code has always had UB errors in it -- please don't solve that by telling it not to warn you about UB.

If you do, though, then please add a configure error stating that the code only builds on i686/amd64, and enforce that the code is always built with -O0, as this will greatly reduce (but not eliminate) the likelihood of things going wrong.

[sisyphus]

The compiler is telling you the code has always had UB errors in it -- please don't solve that by telling it not to warn you about UB.

I don't believe I've suggested in this thread that any warnings be suppressed.

I think I have a patch for POSIX.xs that fixes these issues, but I'll spend some time testing it before posting it here (later today).

[tonycoz]

I suspect we should just be using isnan(...) for double/long double builds ...

Is there a good reason to avoid using isnanl on the long double builds ?

isnan() is standard C99, while isnanl() is a POSIX extension.

I think I have a patch for POSIX.xs that fixes these issues, but I'll spend some time testing it before posting it here (later today).

If you have problems finding the time for this, post here and I'll work on a patch.

[sisyphus]

If you have problems finding the time for this, post here and I'll work on a patch.

Time's not really an issue for me .... can't say the same for "programming capability", unfortunately.
@tonycoz, I'll hand it over to you.

I have managed to get POSIX.xs to build without any need to mess with the flags, using the following patch.

--- ext/POSIX/POSIX.xs_orig     2024-04-27 19:35:14.000000000 +1000
+++ ext/POSIX/POSIX.xs  2024-05-16 13:57:20.939775100 +1000
@@ -13,6 +13,25 @@

 static int not_here(const char *s);

+/* We now declare prototypes to avoid   *
+ * implicit-function-declaration        *
+/* warnings on mingw-w64 windows builds */
+
+#if defined(WIN32) && defined(__GNUC__) && !defined(USE_QUADMATH)
+/* If nvtype is long double, the bessel functions still *
+ * operate at "double precision" only - as in the past. *
+ * Mingw's math.h makes no provision for j0l, y0l, etc. */
+   double j0(double d);
+   double j1(double d);
+   double jn(int n, double d);
+   double y0(double d);
+   double y1(double d);
+   double yn(int n, double d);
+#  if(NVSIZE == 8)
+     int finite(double d);
+#  endif
+#endif
+
 #if defined(PERL_IMPLICIT_SYS)
 #  undef signal
 #  undef open
@@ -1722,7 +1741,7 @@
         newenv = (char*)malloc((strlen(perl_tz_env) + 4) * sizeof(char));
         if (newenv != NULL) {
             sprintf(newenv, "TZ=%s", perl_tz_env);
-            putenv(newenv);
+            _putenv(newenv);
             if (oldenv != NULL)
                 free(oldenv);
             oldenv = newenv;

That, along with my original proposed patch to perl.h (re isnanl) allows 32bit/64bit x double/long double/__float128 nvtypes to build and test perl-5.39.10 fine.
But feel free to make of it what you will.

As a test of the bessel functions on those patched perls I simply ran the following script which checks for "ballpark" results:

use strict;
use warnings;
use Config;

use POSIX qw();

use Test::More;

warn "\n NVTYPE is $Config{nvtype}\n";

my $nv = 11.1234;

# The following values were derived using Math::MPFR,
# and then rounded to 11 decimal places.
# Perl's calculations are typically off by up to a few
# ULPs when compared to the results provided by the
# mpfr library - we settle here for values that are
# "approximately correct".

my $j0 = '-0.14825454045';
my $j1 = '-0.19444418172';
my $jn =  '0.11329325492',
my $y0 = '-0.18760599569';
my $y1 =  '0.13998617519';
my $yn =  '0.21277566957';

cmp_ok(sprintf("%.11g", POSIX::j0($nv)), 'eq', $j0, 'j0(11.234) evaluated ok');
cmp_ok(sprintf("%.11g", POSIX::y0($nv)), 'eq', $y0, 'y0(11.234) evaluated ok');
cmp_ok(sprintf("%.11g", POSIX::j1($nv)), 'eq', $j1, 'j1(11.234) evaluated ok');
cmp_ok(sprintf("%.11g", POSIX::y1($nv)), 'eq', $y1, 'y1(11.234) evaluated ok');
cmp_ok(sprintf("%.11g", POSIX::jn(2, $nv)), 'eq', $jn, 'jn(2, 11.234) evaluated ok');
cmp_ok(sprintf("%.11g", POSIX::yn(2, $nv)), 'eq', $yn, 'yn(2, 11.234) evaluated ok');

done_testing();

That test script is fine on the patched 5.39.10, as it is also on 5.38.0.
But on unpatched 5.39.10 built using gcc-13.2.0 it outputs:

 NVTYPE is double
not ok 1 - j0(11.234) evaluated ok
#   Failed test 'j0(11.234) evaluated ok'
#   at bessel.t line 27.
#          got: '3'
#     expected: '-0.14825454045'
not ok 2 - y0(11.234) evaluated ok
#   Failed test 'y0(11.234) evaluated ok'
#   at bessel.t line 28.
#          got: '3'
#     expected: '-0.18760599569'
not ok 3 - j1(11.234) evaluated ok
#   Failed test 'j1(11.234) evaluated ok'
#   at bessel.t line 29.
#          got: '2'
#     expected: '-0.19444418172'
not ok 4 - y1(11.234) evaluated ok
#   Failed test 'y1(11.234) evaluated ok'
#   at bessel.t line 30.
#          got: '2'
#     expected: '0.13998617519'
not ok 5 - jn(2, 11.234) evaluated ok
#   Failed test 'jn(2, 11.234) evaluated ok'
#   at bessel.t line 31.
#          got: '4'
#     expected: '0.11329325492'
not ok 6 - yn(2, 11.234) evaluated ok
#   Failed test 'yn(2, 11.234) evaluated ok'
#   at bessel.t line 32.
#          got: '2'
#     expected: '0.21277566957'
1..6
# Looks like you failed 6 tests of 6.

I think a test script that performs some tests on the bessel functions should be added to ext/POSIX/t.
(I don't think that script needs to be extensive in its testing.)

As I said at the start of this post, I generally have plenty of time, and running tests is something that I'll happily do.