Remove unnecessary C99< checks for maths functions #4936
Conversation
|
For MSVC (can't speek for other compilers/libraries), this should be fine, since all of these functions are supposed to be supported by VS 2015 and up. This code should be removed as well, though. |
Will remove this piece of code then |
AFAIK we have moved. See b51a99a. |
|
@cmb69 should I modify https://php-lxr.adamharvey.name/source/xref/master/Zend/zend_config.w32.h#53 too? I'm also asking myself if the special |
|
@Girgias, it seems to me that this is needed for C++ compatibility, and as such should not be removed. |
Makes sense will leave it then :) |
| dnl log2 could be used to improve the log function, however it requires C99. The | ||
| dnl check for log2 should be turned on, as soon as we support C99. | ||
| AC_CHECK_FUNCS(asinh acosh atanh log1p hypot) | ||
| AC_FUNC_FNMATCH |
There was a problem hiding this comment.
I'm assuming the removal of AC_FUNC_FNMATCH here was not intentional?
| @@ -422,16 +414,7 @@ AC_RUN_IFELSE([AC_LANG_SOURCE([[ | |||
| #include <math.h> | |||
| #include <stdlib.h> | |||
|
|
|||
| #ifdef HAVE_ISINF | |||
There was a problem hiding this comment.
According to C99 the INFINITY macro should always exist, so it should be possibly to adjust the _zend_get_inf implementation to always use that and drop the two autoconf checks for infinity representations here.
There was a problem hiding this comment.
Similarly for NAN. That one is not guaranteed to exist, but as PHP assumes it's running on a platform with IEEE 754 compliant floats, it should be defined on platforms we target.
There was a problem hiding this comment.
(I would suggest doing this separately, before handling zend_isinf and friends, as it removes some of the more ugly users...)
|
Apart from the AC_FUNC_FNMATCH removal, the asinh etc part here looks fine, can you please land it separately? |
Just to be sure do you want me to split the changes from math.c and the others one into two different commits or should I split every AC check removal into it's own commit? |
|
All the changes for math.c in one commit (feel free to directly commit that as well). |
|
Merged the |
As I'm not sure if we are moving to C99 in master/PHP 8 so I'm putting this up for review.