Hi everyone
We've gotten a bug report about compile errors in the PHP 8.4 alpha on
old C99 compilers (and on some modern compilers when passing the
-std=c99 flag). Specifically, C99 does not support typedef
redeclarations, which is a C11 feature.
// some_header.h
typedef struct _zval_struct zval;
void some_func(zval *zv);
// zend_types.h
struct _zval_struct { ... };
typedef struct _zval_struct zval;
Some headers might want to forward declare zval so that zend_types.h
doesn't have to be included. The two primary reasons you might want to
avoid that are 1. to reduce compile times if the header is very large
and 2. to prevent recursive header dependencies.
However, in C99 this code is actually illegal if both of these headers
end up being included, because redeclarations of typedefs are not
allowed. To fix it, the typedef must be removed and the signatures
must refer to the struct directly.
// some_header.h
struct _zval_struct;
void some_func(struct _zval_struct *zv);
I started fixing these in a PR [1] which required more changes than
expected. After a short discourse, we were wondering whether it might
be better to switch to a newer C standard instead. Our coding
standards [2] currently specify that compiling php-src requires C99.
The Unix installation page on php.net [3] claims it is ANSI C, which
is certainly outdated. There have been suggestions to require C11 for
a while, which should be well supported by all compilers shipped with
maintained distributions.
GCC gained support for C11 in 4.7 [4], LLVM Clang in 3.1, both
released in 2012. For context, Debian Bullseye comes with GCC 10.2
[5], Ubuntu Focal Fossa comes with GCC 9.3 [6], RHEL 8 comes with GCC
8.x [7]. The biggest blocker in the past was MSVC, which has finally
added C11 support in Visual Studio 2019 [8]. Technically, Visual
Studio 2015 and 2017 are still supported by Microsoft [9], but
according to Christoph they are no longer used for PHP builds since
version 8.
Hence, it seems like it would be ok to bump our C compiler requirement
to C11. We'd like to make this change before beta 1 if there are no
objections. There are no immediate plans to make non-optional use of
other C11 features, although that is conceivable at some point.
Another benefit brought up by Christoph is that C11 relegated the
requirement for the compiler to support VLAs (variable-length arrays)
which was never implemented by MSVC, technically not complying with
the currently required standard.
If you do have any concerns, please let me know.
Ilija
[1] https://github.com/php/php-src/pull/15079
[2] https://github.com/php/php-src/blob/8b6f14a9a221599b076e4bc3570f013a7e65aa21/CODING_STANDARDS.md?plain=1#L12
[3] https://www.php.net/manual/en/install.unix.php
[4] https://stackoverflow.com/a/16256596
[5] https://packages.debian.org/bullseye/gcc
[6] https://packages.ubuntu.com/focal/gcc
[7] https://access.redhat.com/solutions/19458
[8] https://devblogs.microsoft.com/cppblog/c11-and-c17-standard-support-arriving-in-msvc/
[9] https://learn.microsoft.com/en-us/visualstudio/productinfo/vs-servicing
Hi everyone
We've gotten a bug report about compile errors in the PHP 8.4 alpha on
old C99 compilers (and on some modern compilers when passing the
-std=c99 flag). Specifically, C99 does not support typedef
redeclarations, which is a C11 feature.// some_header.h typedef struct _zval_struct zval; void some_func(zval *zv); // zend_types.h struct _zval_struct { ... }; typedef struct _zval_struct zval;Some headers might want to forward declare zval so that zend_types.h
doesn't have to be included. The two primary reasons you might want to
avoid that are 1. to reduce compile times if the header is very large
and 2. to prevent recursive header dependencies.However, in C99 this code is actually illegal if both of these headers
end up being included, because redeclarations of typedefs are not
allowed. To fix it, the typedef must be removed and the signatures
must refer to the struct directly.// some_header.h struct _zval_struct; void some_func(struct _zval_struct *zv);I started fixing these in a PR [1] which required more changes than
expected. After a short discourse, we were wondering whether it might
be better to switch to a newer C standard instead. Our coding
standards [2] currently specify that compiling php-src requires C99.
The Unix installation page on php.net [3] claims it is ANSI C, which
is certainly outdated. There have been suggestions to require C11 for
a while, which should be well supported by all compilers shipped with
maintained distributions.GCC gained support for C11 in 4.7 [4], LLVM Clang in 3.1, both
released in 2012. For context, Debian Bullseye comes with GCC 10.2
[5], Ubuntu Focal Fossa comes with GCC 9.3 [6], RHEL 8 comes with GCC
8.x [7].
Even Debian wheezy (out of extended LTS support) or Ubuntu 14.04 have GCC 4.7, so that seems OK.
How about Clang (for OSX) though? I can't check that easily.
cheers
Derick
Hi everyone
We've gotten a bug report about compile errors in the PHP 8.4 alpha on
old C99 compilers (and on some modern compilers when passing the
-std=c99 flag). Specifically, C99 does not support typedef
redeclarations, which is a C11 feature.// some_header.h typedef struct _zval_struct zval; void some_func(zval *zv); // zend_types.h struct _zval_struct { ... }; typedef struct _zval_struct zval;Some headers might want to forward declare zval so that zend_types.h
doesn't have to be included. The two primary reasons you might want to
avoid that are 1. to reduce compile times if the header is very large
and 2. to prevent recursive header dependencies.However, in C99 this code is actually illegal if both of these headers
end up being included, because redeclarations of typedefs are not
allowed. To fix it, the typedef must be removed and the signatures
must refer to the struct directly.// some_header.h struct _zval_struct; void some_func(struct _zval_struct *zv);I started fixing these in a PR [1] which required more changes than
expected. After a short discourse, we were wondering whether it might
be better to switch to a newer C standard instead. Our coding
standards [2] currently specify that compiling php-src requires C99.
The Unix installation page on php.net [3] claims it is ANSI C, which
is certainly outdated. There have been suggestions to require C11 for
a while, which should be well supported by all compilers shipped with
maintained distributions.GCC gained support for C11 in 4.7 [4], LLVM Clang in 3.1, both
released in 2012. For context, Debian Bullseye comes with GCC 10.2
[5], Ubuntu Focal Fossa comes with GCC 9.3 [6], RHEL 8 comes with GCC
8.x [7].Even Debian wheezy (out of extended LTS support) or Ubuntu 14.04 have GCC 4.7, so that seems OK.
How about Clang (for OSX) though? I can't check that easily.
cheers
Derick
Hi,
According to https://releases.llvm.org/3.1/docs/ClangReleaseNotes.html#cchanges, Clang 3.1 added C11.
According to https://trac.macports.org/wiki/XcodeVersionInfo, Clang 3.1 shipped with Xcode 4.3.3, in May 2012.
In terms of confirming this, the oldest macOS VM I have quick access to is running Sierra (from 2016, last OS update in 2019), and default install now for Clang on that VM is 9.0.0
Hope this helps
Cheers
Stephen
According to https://releases.llvm.org/3.1/docs/ClangReleaseNotes.html#cchanges, Clang 3.1 added C11.
According to https://trac.macports.org/wiki/XcodeVersionInfo, Clang 3.1 shipped with Xcode 4.3.3, in May 2012.
In terms of confirming this, the oldest macOS VM I have quick access to is running Sierra (from 2016, last OS update in 2019), and default install now for Clang on that VM is 9.0.0
According to a comment on the pull request[1], the clang version would
be different between LLVM and Apple's fork. An answer on
StackOverflow[2] confirms that. I figure that there is even no exact
matching of the version numbers possible, as Apple might support some
features, but not others. I can even imagine that general C11 support
is available, but not necessarily the typedef redeclaration which is
most important right now. So unless there is some normative
documentation, it might be necessary to see whether PHP 8.4 can be build
with Apple's clang (and which versions).
[1] https://github.com/php/php-src/pull/15079#issuecomment-2263812984
[2]
https://stackoverflow.com/questions/12665744/what-is-the-difference-between-clang-and-apple-clang
Cheers,
Christoph
[…] The biggest blocker in the past was MSVC, which has finally
added C11 support in Visual Studio 2019 [8]. Technically, Visual
Studio 2015 and 2017 are still supported by Microsoft [9], but
according to Christoph they are no longer used for PHP builds since
version 8.
Right. As of PHP 8.0.0, VS16 (aka. Visual Studio 2019) is used for the
builds, and as of PHP 8.4.0, VS17 (aka. Visual Studio 2022) is supposed
to be used. Of course, users may still use an older Visual Studio
version, and want to be able to still build latest PHP, but they still
can install a newer Visual Studio version in parallel (and at least,
using only the Buildtools for Visual Studio 2022, shouldn't be a concern
regarding the license, because they are free to use to build Open-Source
software, and should be totally sufficient to build PHP and extensions).
There might be a slight glitch, namely that a few users want to include
some PHP modules into their software (e.g. the embed SAPI); that might
cause issues, but on the other hand, nobody should expect to work some
latest Open-Source software version to work flawlessly with older
versions of other software. So these users (if there are any) could
still work with PHP 8.3, for instance, until they upgrade to Visual
Studio 2022.
Hence, it seems like it would be ok to bump our C compiler requirement
to C11. We'd like to make this change before beta 1 if there are no
objections. There are no immediate plans to make non-optional use of
other C11 features, although that is conceivable at some point.
I'm all for it, given the alternatives would be worse, and presuming
there is no real show-stopper.
Another benefit brought up by Christoph is that C11 relegated the
requirement for the compiler to support VLAs (variable-length arrays)
which was never implemented by MSVC, technically not complying with
the currently required standard.
Indeed, that caused a couple of issues in the past, not only for
php-src, but also for extensions. Requiring C11 support might help
developers to detect that early, and choose an alternative
implementation right away.
[8] https://devblogs.microsoft.com/cppblog/c11-and-c17-standard-support-arriving-in-msvc/
[9] https://learn.microsoft.com/en-us/visualstudio/productinfo/vs-servicing
Cheers,
Christoph
[...]
I started fixing these in a PR [1] which required more changes than
expected. After a short discourse, we were wondering whether it might
be better to switch to a newer C standard instead. Our coding
standards [2] currently specify that compiling php-src requires C99.
The Unix installation page on php.net [3] claims it is ANSI C, which
is certainly outdated. There have been suggestions to require C11 for
a while, which should be well supported by all compilers shipped with
maintained distributions.
It feels wrong to raise such an important requirement that might affect a
lot of people, including maintainers of extensions, for just one specific
problem, and 99% of the codebase would still be C99 compliant.
I quickly put together an alternative PR (#15202) with a slightly different
approach, just as a proof of concept. The idea is to move all the global
typedefs in a new include header "zend_types_defs.h" (but also
zend_portability.h can be reused for this purpose, as all the relevant
files already include it).
While putting together that PR, I had the feeling that this typedef
redefinition problem is in reality hiding some smelly design of header
files. So maybe, rather than requiring a compiler more tolerant to poor
code, we should rather focus on getting the design right.
Hi,
On Fri, Aug 2, 2024 at 1:36 PM Giovanni Giacobbi giovanni@giacobbi.net
wrote:
It feels wrong to raise such an important requirement that might affect a
lot of people, including maintainers of extensions
Why do you think that this might affect a lot of people? We are talking
about minimum GCC version 4.7 and really old Clang version (that probably
no one uses) as well. And minimum MSVC is already 2019. Do you know about
some specific platforms that would be affected by this?
Cheers
Jakub
[...]
I started fixing these in a PR [1] which required more changes than
expected. After a short discourse, we were wondering whether it might
be better to switch to a newer C standard instead. Our coding
standards [2] currently specify that compiling php-src requires C99.
The Unix installation page on php.net [3] claims it is ANSI C, which
is certainly outdated. There have been suggestions to require C11 for
a while, which should be well supported by all compilers shipped with
maintained distributions.It feels wrong to raise such an important requirement that might affect a lot of people, including maintainers of extensions, for just one specific problem, and 99% of the codebase would still be C99 compliant.
I quickly put together an alternative PR (#15202) with a slightly different approach, just as a proof of concept. The idea is to move all the global typedefs in a new include header "zend_types_defs.h" (but also zend_portability.h can be reused for this purpose, as all the relevant files already include it).
While putting together that PR, I had the feeling that this typedef redefinition problem is in reality hiding some smelly design of header files. So maybe, rather than requiring a compiler more tolerant to poor code, we should rather focus on getting the design right.
I would like to see actual factual data rather than "vibes" as to how this would cause issues with third-party extensions.
php-src is not C99 pedeantic compliant and we do use GCC extenstions.
I do agree that maybe we should go back to fixing headers, but considering the drama this caused 18 months ago I do not know who here has the motivation to do this.
Ideally I feel we should target C17 as it is a "bug fix" release of C11, but this might be impossible due to old compilers still being the default on LTS Distributions.
The main benefit of C11/17 in the long run are atomics, that we kinda use already anyway.
Moreover, as someone that has written an extension that has some usage, when I go back to it, I'd rather like to be able to use features from C23 than be stuck on C99, but that is just me.
Best regards,
Gina P. Banyard
[...]
I started fixing these in a PR [1] which required more changes than
expected. After a short discourse, we were wondering whether it might
be better to switch to a newer C standard instead. Our coding
standards [2] currently specify that compiling php-src requires C99.
The Unix installation page on php.net [3] claims it is ANSI C, which
is certainly outdated. There have been suggestions to require C11 for
a while, which should be well supported by all compilers shipped with
maintained distributions.It feels wrong to raise such an important requirement that might affect a lot of people, including maintainers of extensions, for just one specific problem, and 99% of the codebase would still be C99 compliant.
I quickly put together an alternative PR (#15202) with a slightly different approach, just as a proof of concept. The idea is to move all the global typedefs in a new include header "zend_types_defs.h" (but also zend_portability.h can be reused for this purpose, as all the relevant files already include it).
While putting together that PR, I had the feeling that this typedef redefinition problem is in reality hiding some smelly design of header files. So maybe, rather than requiring a compiler more tolerant to poor code, we should rather focus on getting the design right.
I would like to see actual factual data rather than "vibes" as to how this would cause issues with third-party extensions.
php-src is not C99 pedeantic compliant and we do use GCC extenstions.I do agree that maybe we should go back to fixing headers, but considering the drama this caused 18 months ago I do not know who here has the motivation to do this.
Ideally I feel we should target C17 as it is a "bug fix" release of C11, but this might be impossible due to old compilers still being the default on LTS Distributions.
The main benefit of C11/17 in the long run are atomics, that we kinda use already anyway.
As a person who wrote some of those atomics, I would definitely prefer
to move to standard C11/C17 atomics at some point in the future and
remove all other fallbacks. The blocker right now is Microsoft Visual
Studio. Although they added C11/C17 support a few versions back, they
did not add atomics at that time. It's available now, but it is still
experimental as far as I can tell.
Maybe for PHP 9.0 we can address header issues and also by then VS
will have a general release we can move to for atomics.
One thing I feel strongly about is that we should not add flags like
-std=c11. If we check, we ought to just ensure that whatever flags
the user provided, we have certain C11/C17 features. This allows for
extensions to use a newer version, or a relaxed version like
-std=gnu11.
As a person who wrote some of those atomics, I would definitely prefer
to move to standard C11/C17 atomics at some point in the future and
remove all other fallbacks. The blocker right now is Microsoft Visual
Studio. Although they added C11/C17 support a few versions back, they
did not add atomics at that time. It's available now, but it is still
experimental as far as I can tell.
There is a MS blog post from December, 2022[1] which claims that it is
experimental, and that you would need to specify the
/experimental:c11atomics flag in addition to /std:c11 to enable atomics
support. I though, hey, why not try that out, added only /std:c11 and
got a failing minimal build, because apparently max_align_t is not
defined in stddef.h even with the latest available SDK (10.0.26100.0)
offered by Visual Studio 2022 for my Windows 10 machine. It might be
available with SDK 10.0.22621.2428, which according to the
documentation[2] has been "Relesed in October 2024"[sic]. I wonder
whether I would trust more detailed information there.
(FWIW, the official Windows builds use 10.0.22621.0, and we probably
should enforce some common version; I think for now we use just what can
be found in the build environment.)
Anyway, I worked around the missing max_align_t definition, and the
build succeeded, even though I've added
#ifndef STDC_NO_ATOMICS
error cmb
#endif
in zend_API.c. Thus, I conclude that atomics support in latest Visual
Studio 17.10.5 is no longer deemed experimental, and locking atomics are
implemented now. I think it is worth investigating whether that support
actually works for our purposes, but probably not use that for PHP 8.4.
[1]
https://devblogs.microsoft.com/cppblog/c11-atomics-in-visual-studio-2022-version-17-5-preview-2/
[2] https://developer.microsoft.com/en-us/windows/downloads/sdk-archive/
Christoph
Hence, it seems like it would be ok to bump our C compiler requirement
to C11. We'd like to make this change before beta 1 if there are no
objections.
How would you enforce that, through configure?
FWIW, I have just tried compiling Xdebug with -std=c2x (C23) and that
caused me one trivial required change:
https://github.com/xdebug/xdebug/pull/969/files
cheers,
Derick
Hence, it seems like it would be ok to bump our C compiler requirement
to C11. We'd like to make this change before beta 1 if there are no
objections.How would you enforce that, through configure?
In my opinion, there is no need to enforce this requirement. Just
having documentation should be sufficient. Users with an old compilerr
will notice issues (i.e. build failures), and even if they don't care to
look up the docs, they will file a ticket, and that can quickly be
closed pointing to the relevant documentation.
Christoph
Hence, it seems like it would be ok to bump our C compiler
requirement to C11. We'd like to make this change before beta 1 if
there are no objections.How would you enforce that, through configure?
In my opinion, there is no need to enforce this requirement. Just
having documentation should be sufficient. Users with an old
compilerr will notice issues (i.e. build failures), and even if they
don't care to look up the docs, they will file a ticket, and that can
quickly be closed pointing to the relevant documentation.
Instead of having to deal with tickets, wouldn't be be easier if the
compiler they used works with the features that we are using? That could
instantly provide a link to the documentation saving all of us time.
cheers,
Derick
--
https://derickrethans.nl | https://xdebug.org | https://dram.io
Author of Xdebug. Like it? Consider supporting me: https://xdebug.org/support
mastodon: @derickr@phpc.social @xdebug@phpc.social
Instead of having to deal with tickets, wouldn't be be easier if the
compiler they used works with the features that we are using? That could
instantly provide a link to the documentation saving all of us time.
it is significantly easier at configure time. Gcc 4.9 is (almost) fully c11
compliant (not 4.7).
As of windows, it will be dependent on the vc version used by the php minor
branch where it may be introduced.
That being said, I am not sure if the reason mentioned for using c11 in the
initial post is used in external extensions (included headers etc), but at
least on linux (or similar), an ext can be compiled with any version.
On windows, we can't, mainly for memory management issues (alloc/realloc
or freed using different crt). It is possible but too many extensions, also
in core, cause issues.
best,
Pierre
Instead of having to deal with tickets, wouldn't be be easier if the
compiler they used works with the features that we are using? That could
instantly provide a link to the documentation saving all of us time.it is significantly easier at configure time. Gcc 4.9 is (almost) fully c11
compliant (not 4.7).
But what about other compilers we support on non Windows platforms
currently, like clang, Apple's clang, Solaris Studio and maybe some more
we don't even know about.
As of windows, it will be dependent on the vc version used by the php minor
branch where it may be introduced.
It's supposed to be introduced as of PHP 8.4.0, which already uses VS
17, although MSVC supports C11 as of VS 16.5 (while it never claimed to
support C99). So in practise, even those still building their own
Windows binaries with Visual Studio 2019 should not be affected.
That being said, I am not sure if the reason mentioned for using c11 in the
initial post is used in external extensions (included headers etc), but at
least on linux (or similar), an ext can be compiled with any version.
I somewhat doubt that; we're using a couple of C99 features (like mixed
declarations and statements) in the core for a couple of years (IIRC, as
of PHP 8.0.0), and extensions may do this with their own code, or might
be forced to include public php-src headers which may use some C99
features. This may not be an issue with any C compilers actually still
in use, but can be sure about that?
And some extensions even require C++ support.
Christoph
Instead of having to deal with tickets, wouldn't be be easier if
the compiler they used works with the features that we are using?
That could instantly provide a link to the documentation saving all
of us time.it is significantly easier at configure time. Gcc 4.9 is (almost)
fully c11 compliant (not 4.7).But what about other compilers we support on non Windows platforms
currently, like clang, Apple's clang, Solaris Studio and maybe some
more we don't even know about.
Normal clang supports the same flag. Apple clang potentially too, but
not currently:
https://opensource.apple.com/source/clang/clang-23/clang/tools/clang/docs/UsersManual.html
— it could be outdated information though. I don't have a mac to test.
But it's just possible to check for the flag for compilers that we do
know about. And by checking for it and showing a warning, we make
people's lives easire.
cheers,
Derick
Normal clang supports the same flag. Apple clang potentially too, but
not currently:
https://opensource.apple.com/source/clang/clang-23/clang/tools/clang/docs/UsersManual.html
— it could be outdated information though. I don't have a mac to test.But it's just possible to check for the flag for compilers that we do
know about. And by checking for it and showing a warning, we make
people's lives easire.
Not sure about which flag you are talking, but anyway see
https://news-web.php.net/php.internals/124775. I.e. I'm fine with a
feature test, but wouldn't want to check for individual compiler
versions (although I have no hard feelings about that).
Christoph
Normal clang supports the same flag. Apple clang potentially too, but
not currently:
https://opensource.apple.com/source/clang/clang-23/clang/tools/clang/docs/UsersManual.html
— it could be outdated information though. I don't have a mac to test.
I believe that is extrmely outdated, based on the other listings on https://opensource.apple.com/source/clang/ and the listings by macOS version on https://opensource.apple.com/releases/
To be certain, I fired up my old macOS Sierra VM again (which reports as having Clang 9) and tried a simple clang -std=c11 foo.c - it doesn't complain about c11 being invalid (which it does if I try say -std=c17)
Is there a specific branch in the repo (or in a fork) that already uses the required features? I can try to compile on this VM, to confirm whether it actually works.
Cheers
Stephen
But what about other compilers we support on non Windows platforms
currently, like clang, Apple's clang, Solaris Studio and maybe some more
we don't even know about.
Might be worth noting here that from what I've tested so far that PHP
cannot be built currently with Solaris Studio compiler and most likely
any other compiler out of the range of MSVC, GCC and Clang (including
the AppleClang). There are errors in the configure steps due to not
properly checked linker flags and there are lots of warnings and
errors produced during the make step. So, basically the list of
"supported" and heavily tested compilers is at this point: GCC and
Clang.
I find this strange, but it also makes sense to not be able to support
a wide variety of compilers without having additional CI builds.
A nice list of "relevant" compilers from the build system point of
view for some overview:
https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_COMPILER_ID.html
But what about other compilers we support on non Windows platforms
currently, like clang, Apple's clang, Solaris Studio and maybe some more
we don't even know about.Might be worth noting here that from what I've tested so far that PHP
cannot be built currently with Solaris Studio compiler and most likely
any other compiler out of the range of MSVC, GCC and Clang (including
the AppleClang). There are errors in the configure steps due to not
properly checked linker flags and there are lots of warnings and
errors produced during the make step. So, basically the list of
"supported" and heavily tested compilers is at this point: GCC and
Clang.
Oh! I wasn't aware of that.
I find this strange, but it also makes sense to not be able to support
a wide variety of compilers without having additional CI builds.
Well, users of other compilers could file bug reports, and we could
possibly address them even without setting up another CI build (it seems
those not using Github runners are already not really helpful).
A nice list of "relevant" compilers from the build system point of
view for some overview:
https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_COMPILER_ID.html
Interesting, thanks! While I would not have expected that php-src can
be built on all of these, at least I thought some of these were supported.
Christoph
Instead of having to deal with tickets, wouldn't be be easier if the
compiler they used works with the features that we are using? That could
instantly provide a link to the documentation saving all of us time.it is significantly easier at configure time. Gcc 4.9 is (almost) fully c11
compliant (not 4.7).But what about other compilers we support on non Windows platforms
currently, like clang, Apple's clang, Solaris Studio and maybe some more
we don't even know about.
Sorry, I meant to actually check if that feature is supported with
AC_COMPILE_*. It is painful but has proven to be the only reliable
way.
As gcc or VCC can be slightly more trusted using their versions, the
other, especially on MacOs, can't be trusted, ever. Let alone on other
OSes I still wonder why we support them for latest php 8 versions
(solaris f.e.).
As of windows, it will be dependent on the vc version used by the php minor
branch where it may be introduced.It's supposed to be introduced as of PHP 8.4.0, which already uses VS
17, although MSVC supports C11 as of VS 16.5 (while it never claimed to
support C99). So in practise, even those still building their own
Windows binaries with Visual Studio 2019 should not be affected.That being said, I am not sure if the reason mentioned for using c11 in the
initial post is used in external extensions (included headers etc), but at
least on linux (or similar), an ext can be compiled with any version.I somewhat doubt that; we're using a couple of C99 features (like mixed
declarations and statements) in the core for a couple of years (IIRC, as
of PHP 8.0.0), and extensions may do this with their own code, or might
be forced to include public php-src headers which may use some C99
features. This may not be an issue with any C compilers actually still
in use, but can be sure about that?
We can't :).
To my understanding the challenges are the extensions maintainers. On
Windows it is a controlled environment. We had similar discussions
back then when we dropped VC6, which was a much bigger step. I would
prefer to go with what makes your work easier than trying to keep
older VC support for the latest release, in case we decide to support
a minimum VC version for valid reasons. atomic support seems to be
one, the header one, I am very doubtful about it as something worth
the troubles :). This is being only about windows, that's indeed not
valid for any linux/*bsd/*wrt/etc.
cheers,
Pierre
@pierrejoye | http://www.libgd.org
In my opinion, there is no need to enforce this requirement. Just
having documentation should be sufficient. Users with an old
compilerr will notice issues (i.e. build failures), and even if they
don't care to look up the docs, they will file a ticket, and that can
quickly be closed pointing to the relevant documentation.Instead of having to deal with tickets, wouldn't be be easier if the
compiler they used works with the features that we are using? That could
instantly provide a link to the documentation saving all of us time.
Note that I'm not against checking C11 support during configure. If
there is a respective feature test readily available for autoconf –
great. Otherwise it might be sufficient to check for the definition and
value of STDC_VERSION. However, I don't think it would be a good
idea to test for certain compilers and their versions; that list might
need more maintaince than catering to a couple of bug reports. Before
we go down that route, we may be better off just documenting the
requirement.
Christoph
Hi everyone
We've gotten a bug report about compile errors in the PHP 8.4 alpha on
old C99 compilers (and on some modern compilers when passing the
-std=c99 flag). Specifically, C99 does not support typedef
redeclarations, which is a C11 feature.// some_header.h typedef struct _zval_struct zval; void some_func(zval *zv); // zend_types.h struct _zval_struct { ... }; typedef struct _zval_struct zval;Some headers might want to forward declare zval so that zend_types.h
doesn't have to be included. The two primary reasons you might want to
avoid that are 1. to reduce compile times if the header is very large
and 2. to prevent recursive header dependencies.However, in C99 this code is actually illegal if both of these headers
end up being included, because redeclarations of typedefs are not
allowed. To fix it, the typedef must be removed and the signatures
must refer to the struct directly.// some_header.h struct _zval_struct; void some_func(struct _zval_struct *zv);I started fixing these in a PR [1] which required more changes than
expected. After a short discourse, we were wondering whether it might
be better to switch to a newer C standard instead. Our coding
standards [2] currently specify that compiling php-src requires C99.
The Unix installation page on php.net [3] claims it is ANSI C, which
is certainly outdated. There have been suggestions to require C11 for
a while, which should be well supported by all compilers shipped with
maintained distributions.GCC gained support for C11 in 4.7 [4], LLVM Clang in 3.1, both
released in 2012. For context, Debian Bullseye comes with GCC 10.2
[5], Ubuntu Focal Fossa comes with GCC 9.3 [6], RHEL 8 comes with GCC
8.x [7]. The biggest blocker in the past was MSVC, which has finally
added C11 support in Visual Studio 2019 [8]. Technically, Visual
Studio 2015 and 2017 are still supported by Microsoft [9], but
according to Christoph they are no longer used for PHP builds since
version 8.Hence, it seems like it would be ok to bump our C compiler requirement
to C11. We'd like to make this change before beta 1 if there are no
objections. There are no immediate plans to make non-optional use of
other C11 features, although that is conceivable at some point.Another benefit brought up by Christoph is that C11 relegated the
requirement for the compiler to support VLAs (variable-length arrays)
which was never implemented by MSVC, technically not complying with
the currently required standard.If you do have any concerns, please let me know.
Ilija
[1] https://github.com/php/php-src/pull/15079
[2]
https://github.com/php/php-src/blob/8b6f14a9a221599b076e4bc3570f013a7e65aa21/CODING_STANDARDS.md?plain=1#L12
[3] https://www.php.net/manual/en/install.unix.php
[4] https://stackoverflow.com/a/16256596
[5] https://packages.debian.org/bullseye/gcc
[6] https://packages.ubuntu.com/focal/gcc
[7] https://access.redhat.com/solutions/19458
[8]
https://devblogs.microsoft.com/cppblog/c11-and-c17-standard-support-arriving-in-msvc/
[9]
https://learn.microsoft.com/en-us/visualstudio/productinfo/vs-servicing
Would now be a good time to consider switching to cmake? Peter(petk) has a
cmake build system on his github that just needs to be touched up.
Hence, it seems like it would be ok to bump our C compiler requirement
to C11. We'd like to make this change before beta 1 if there are no
objections. There are no immediate plans to make non-optional use of
other C11 features, although that is conceivable at some point.Would now be a good time to consider switching to cmake? Peter(petk) has a
cmake build system on his github that just needs to be touched up.
I would suggest to start an own thread about this, since these appear to
be mostly unrelated issues.
Christoph
Hence, it seems like it would be ok to bump our C compiler requirement
to C11. We'd like to make this change before beta 1 if there are no
objections. There are no immediate plans to make non-optional use of
other C11 features, although that is conceivable at some point.
So skimming the whole discussion[1] it seems that most are generally
fine with bumping the requirements to C11, except for Giovanni Giacobbi
(whose draft PR[2] had no further discussion so far), and maybe for some
uncertainties regarding some less used compilers.
Given that feature freeze is in 3 days[3], it appears to be prudent to
have a final decision now, and to close PR #15079[4] if we're going with
C11, or merge that or Giovanni's PR[2], otherwise.
And if we're going with C11, figuring out the details (which
configuration check to use, or only documenting the requirement) can
still be decided somehwat later, in my opinion.
[1] https://externals.io/message/124706
[2] https://github.com/php/php-src/pull/15202
[3] https://wiki.php.net/todo/php84#timetable
[4] https://github.com/php/php-src/pull/15079
Cheers,
Christoph
Hi Christoph
So skimming the whole discussion[1] it seems that most are generally
fine with bumping the requirements to C11, except for Giovanni Giacobbi
(whose draft PR[2] had no further discussion so far), and maybe for some
uncertainties regarding some less used compilers.
Giovanni's remark that this would impact many people was challenged by
Jakub [1] which didn't get a response. I believe it's safe to assume
that this isn't the case.
And if we're going with C11, figuring out the details (which
configuration check to use, or only documenting the requirement) can
still be decided somehwat later, in my opinion.
The consensus seems to be that bumping to C11 is ok. While keeping C99
compatibility wouldn't be a big hassle, it doesn't seem necessary
anymore.
I would agree with Christoph that documentation is enough for now. If
your compiler happens to support all the C11 features we require,
without full C11 compliance, then that's ok. If we want to reduce bug
reports, a configure check for typedef redeclarations, nudging people
in the right direction with an appropriate message might help.
Verifying C11 support might be tricky, given that the compiler flags
are not standardized. But I'm not a build system expert, there might
be an approach I'm not aware of.
Ilija
Giovanni's remark that this would impact many people was challenged by
Jakub [1] which didn't get a response. I believe it's safe to assume
that this isn't the case.
I somehow forgot to link this reference, sorry about that.
Hi Christoph
On Sat, Aug 10, 2024 at 2:19 PM Christoph M. Becker cmbecker69@gmx.de
wrote:So skimming the whole discussion[1] it seems that most are generally
fine with bumping the requirements to C11, except for Giovanni Giacobbi
(whose draft PR[2] had no further discussion so far), and maybe for some
uncertainties regarding some less used compilers.Giovanni's remark that this would impact many people was challenged by
Jakub [1] which didn't get a response. I believe it's safe to assume
that this isn't the case.
I'd like to remark that this is not a decision about renouncing some
valuable feature in order to support older compilers, but rather about
applying a patch shy of ~100 lines where half of the changes should be
considered bugfixes.
If you think it would be acceptable to have an additional header file
pre-declaring the typedefs, it would solve the only standing issue that
prevents the entire codebase to be C99 compliant. Isn't it worth it? The
more systems that can bild PHP, the better :-)
Also this is not a binding decision: If in the future there is a real need
to increase the requirement to C11, you can always do that.
Hi Christoph
So skimming the whole discussion[1] it seems that most are generally
fine with bumping the requirements to C11, except for Giovanni Giacobbi
(whose draft PR[2] had no further discussion so far), and maybe for some
uncertainties regarding some less used compilers.Giovanni's remark that this would impact many people was challenged by
Jakub [1] which didn't get a response. I believe it's safe to assume
that this isn't the case.I'd like to remark that this is not a decision about renouncing some valuable feature in order to support older compilers, but rather about applying a patch shy of ~100 lines where half of the changes should be considered bugfixes.
If you think it would be acceptable to have an additional header file pre-declaring the typedefs, it would solve the only standing issue that prevents the entire codebase to be C99 compliant. Isn't it worth it? The more systems that can bild PHP, the better :-)
Also this is not a binding decision: If in the future there is a real need to increase the requirement to C11, you can always do that.
Even with this change we would not be fully C99 compliant, as we do rely on GCC extensions AFAIK.
Moreover, a bunch of us want to move to C11/C17.
C11 is a standard that is 13 years old, and C99 is 25 years old, I think it is totally reasonable for us to bump the language spec we rely on.
Finally, I would like to, once again, see a system where moving to C11 is an actual problem.
Best regards,
Gina P. Banyard
Hence, it seems like it would be ok to bump our C compiler requirement
to C11. We'd like to make this change before beta 1 if there are no
objections. There are no immediate plans to make non-optional use of
other C11 features, although that is conceivable at some point.So skimming the whole discussion[1] it seems that most are generally
fine with bumping the requirements to C11, except for Giovanni Giacobbi
(whose draft PR[2] had no further discussion so far), and maybe for some
uncertainties regarding some less used compilers.Given that feature freeze is in 3 days[3], it appears to be prudent to
have a final decision now, and to close PR #15079[4] if we're going with
C11, or merge that or Giovanni's PR[2], otherwise.And if we're going with C11, figuring out the details (which
configuration check to use, or only documenting the requirement) can
still be decided somehwat later, in my opinion.[1] https://externals.io/message/124706
[2] https://github.com/php/php-src/pull/15202
[3] https://wiki.php.net/todo/php84#timetable
[4] https://github.com/php/php-src/pull/15079Cheers,
Christoph
Given the timetable, I wouldn't change the C std requirements for 8.4.
I would stop relying on the typedef and forward declare only the
struct, and that. Note that although Windows supports C11, it does not
support all features including atomics. Someone chimed in to say that
they are available, but this doesn't match the information I got from
a coworker who did a similar test. Given conflicting information and
the short timetable, I think we should lean towards being cautious. I
hope for 8.5/9.0 we can move to C11/C17 which can improve the typedef
situation, simplify our atomics handling, and more.
Hi Levi
On Mon, Aug 12, 2024 at 5:56 PM Levi Morrison
levi.morrison@datadoghq.com wrote:
Given the timetable, I wouldn't change the C std requirements for 8.4.
Just to state it officially: You object to switching to C11 in 8.4? In
that case, we'll have to postpone.
I would stop relying on the typedef and forward declare only the
struct, and that. Note that although Windows supports C11, it does not
support all features including atomics. Someone chimed in to say that
they are available, but this doesn't match the information I got from
a coworker who did a similar test. Given conflicting information and
the short timetable, I think we should lean towards being cautious. I
hope for 8.5/9.0 we can move to C11/C17 which can improve the typedef
situation, simplify our atomics handling, and more.
I'm a bit confused about the relevance of C11 atomics. As I'm sure
you're aware, they remain optional in C17/C23. So, we'll need to
support a fallback, my suggestion was not to remove the fallback.
Essentially, code-wise, nothing would change if we adopt C11, except
being allowed to redeclare typedefs. Apart from that, only the
documentation would change.
Ilija
Hi Levi
On Mon, Aug 12, 2024 at 5:56 PM Levi Morrison
levi.morrison@datadoghq.com wrote:Given the timetable, I wouldn't change the C std requirements for 8.4.
Just to state it officially: You object to switching to C11 in 8.4? In
that case, we'll have to postpone.
I don't object. I think it's smarter to wait, and do more than just
redeclare a few typedefs. But I won't block it.
I would stop relying on the typedef and forward declare only the
struct, and that. Note that although Windows supports C11, it does not
support all features including atomics. Someone chimed in to say that
they are available, but this doesn't match the information I got from
a coworker who did a similar test. Given conflicting information and
the short timetable, I think we should lean towards being cautious. I
hope for 8.5/9.0 we can move to C11/C17 which can improve the typedef
situation, simplify our atomics handling, and more.I'm a bit confused about the relevance of C11 atomics. As I'm sure
you're aware, they remain optional in C17/C23. So, we'll need to
support a fallback, my suggestion was not to remove the fallback.Essentially, code-wise, nothing would change if we adopt C11, except
being allowed to redeclare typedefs. Apart from that, only the
documentation would change.
If only documentation changes, then I think the risk is lower. We
definitely should not be doing configure checks in 8.4 with how little
time there is, except maybe one that attempts to redeclare a typedef
to see if it compiles, and give a nicer error message.
Hi Levi
On Tue, Aug 13, 2024 at 5:00 PM Levi Morrison
levi.morrison@datadoghq.com wrote:
Just to state it officially: You object to switching to C11 in 8.4? In
that case, we'll have to postpone.I don't object. I think it's smarter to wait, and do more than just
redeclare a few typedefs. But I won't block it.
Thank you for the clarification. In that case, I'd say let's go
forward with the proposal. I agree that we should be making use of
more C11 features in the future, once the opportunity presents itself.
If only documentation changes, then I think the risk is lower. We
definitely should not be doing configure checks in 8.4 with how little
time there is, except maybe one that attempts to redeclare a typedef
to see if it compiles, and give a nicer error message.
I agree. I will create a PR to provide a ./configure time error
message if the compiler doesn't support typedef redeclarations.
Adopting C11 does mean we can start using other C11 features too,
though that should naturally happen only on (future) master rather
than 8.4, given that ABI freeze is only a little over a month away.
Ilija