Skip to content

Resolve MSVC C4244 level 2 warnings #17076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Resolve MSVC C4244 level 2 warnings #17076

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0cee428
Resolve MSVC C4244 level 2 warnings
cmb69 Dec 7, 2024
8e54ece
fix merge-conflict
cmb69 Dec 9, 2024
91c6818
fix mbstring
cmb69 Dec 9, 2024
e8204d0
Merge branch 'master' into cmb/c4244
cmb69 Dec 10, 2024
981c8a0
Merge branch 'master' into cmb/c4244
cmb69 Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/scripts/windows/build_task.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ if %errorlevel% neq 0 exit /b 3
if "%THREAD_SAFE%" equ "0" set ADD_CONF=%ADD_CONF% --disable-zts
if "%INTRINSICS%" neq "" set ADD_CONF=%ADD_CONF% --enable-native-intrinsics=%INTRINSICS%

set CFLAGS=/W1 /WX /w14013
set CFLAGS=/W2 /WX /w14013 /wd4146 /wd4305

cmd /c configure.bat ^
--enable-snapshot-build ^
Expand Down
18 changes: 9 additions & 9 deletions ext/ffi/ffi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7496,10 +7496,10 @@ void zend_ffi_expr_cast(zend_ffi_val *val, zend_ffi_dcl *dcl) /* {{{ */
case ZEND_FFI_TYPE_FLOAT:
if (val->kind == ZEND_FFI_VAL_UINT32 || val->kind == ZEND_FFI_VAL_UINT64) {
val->kind = ZEND_FFI_VAL_FLOAT;
val->d = val->u64;
val->d = (zend_ffi_double) val->u64;
} else if (val->kind == ZEND_FFI_VAL_INT32 || val->kind == ZEND_FFI_VAL_INT64) {
val->kind = ZEND_FFI_VAL_FLOAT;
val->d = val->i64;
val->d = (zend_ffi_double) val->i64;
} else if (val->kind == ZEND_FFI_VAL_FLOAT || val->kind == ZEND_FFI_VAL_DOUBLE || val->kind == ZEND_FFI_VAL_LONG_DOUBLE) {
val->kind = ZEND_FFI_VAL_FLOAT;
} else if (val->kind == ZEND_FFI_VAL_CHAR) {
Expand All @@ -7512,10 +7512,10 @@ void zend_ffi_expr_cast(zend_ffi_val *val, zend_ffi_dcl *dcl) /* {{{ */
case ZEND_FFI_TYPE_DOUBLE:
if (val->kind == ZEND_FFI_VAL_UINT32 || val->kind == ZEND_FFI_VAL_UINT64) {
val->kind = ZEND_FFI_VAL_DOUBLE;
val->d = val->u64;
val->d = (zend_ffi_double) val->u64;
} else if (val->kind == ZEND_FFI_VAL_INT32 || val->kind == ZEND_FFI_VAL_INT64) {
val->kind = ZEND_FFI_VAL_DOUBLE;
val->d = val->i64;
val->d = (zend_ffi_double) val->i64;
} else if (val->kind == ZEND_FFI_VAL_FLOAT || val->kind == ZEND_FFI_VAL_DOUBLE || val->kind == ZEND_FFI_VAL_LONG_DOUBLE) {
val->kind = ZEND_FFI_VAL_DOUBLE;
} else if (val->kind == ZEND_FFI_VAL_CHAR) {
Expand Down Expand Up @@ -7551,7 +7551,7 @@ void zend_ffi_expr_cast(zend_ffi_val *val, zend_ffi_dcl *dcl) /* {{{ */
val->kind = ZEND_FFI_VAL_UINT32;
} else if (val->kind == ZEND_FFI_VAL_FLOAT || val->kind == ZEND_FFI_VAL_DOUBLE || val->kind == ZEND_FFI_VAL_LONG_DOUBLE) {
val->kind = ZEND_FFI_VAL_UINT32;
val->u64 = val->d;
val->u64 = (uint64_t) val->d;
} else if (val->kind == ZEND_FFI_VAL_CHAR) {
val->kind = ZEND_FFI_VAL_UINT32;
val->u64 = val->ch;
Expand All @@ -7566,7 +7566,7 @@ void zend_ffi_expr_cast(zend_ffi_val *val, zend_ffi_dcl *dcl) /* {{{ */
val->kind = ZEND_FFI_VAL_INT32;
} else if (val->kind == ZEND_FFI_VAL_FLOAT || val->kind == ZEND_FFI_VAL_DOUBLE || val->kind == ZEND_FFI_VAL_LONG_DOUBLE) {
val->kind = ZEND_FFI_VAL_INT32;
val->i64 = val->d;
val->i64 = (uint64_t) val->d;
} else if (val->kind == ZEND_FFI_VAL_CHAR) {
val->kind = ZEND_FFI_VAL_INT32;
val->i64 = val->ch;
Expand All @@ -7579,7 +7579,7 @@ void zend_ffi_expr_cast(zend_ffi_val *val, zend_ffi_dcl *dcl) /* {{{ */
val->kind = ZEND_FFI_VAL_UINT64;
} else if (val->kind == ZEND_FFI_VAL_FLOAT || val->kind == ZEND_FFI_VAL_DOUBLE || val->kind == ZEND_FFI_VAL_LONG_DOUBLE) {
val->kind = ZEND_FFI_VAL_UINT64;
val->u64 = val->d;
val->u64 = (uint64_t) val->d;
} else if (val->kind == ZEND_FFI_VAL_CHAR) {
val->kind = ZEND_FFI_VAL_UINT64;
val->u64 = val->ch;
Expand All @@ -7596,7 +7596,7 @@ void zend_ffi_expr_cast(zend_ffi_val *val, zend_ffi_dcl *dcl) /* {{{ */
val->ch = val->i64;
} else if (val->kind == ZEND_FFI_VAL_FLOAT || val->kind == ZEND_FFI_VAL_DOUBLE || val->kind == ZEND_FFI_VAL_LONG_DOUBLE) {
val->kind = ZEND_FFI_VAL_CHAR;
val->ch = val->d;
val->ch = (char) val->d;
} else if (val->kind == ZEND_FFI_VAL_CHAR) {
} else {
val->kind = ZEND_FFI_VAL_ERROR;
Expand All @@ -7607,7 +7607,7 @@ void zend_ffi_expr_cast(zend_ffi_val *val, zend_ffi_dcl *dcl) /* {{{ */
val->kind = ZEND_FFI_VAL_UINT32;
} else if (val->kind == ZEND_FFI_VAL_FLOAT || val->kind == ZEND_FFI_VAL_DOUBLE || val->kind == ZEND_FFI_VAL_LONG_DOUBLE) {
val->kind = ZEND_FFI_VAL_UINT32;
val->u64 = val->d;
val->u64 = (uint64_t) val->d;
} else if (val->kind == ZEND_FFI_VAL_CHAR) {
val->kind = ZEND_FFI_VAL_UINT32;
val->u64 = val->ch;
Expand Down
6 changes: 3 additions & 3 deletions ext/gd/gd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4017,7 +4017,7 @@ PHP_FUNCTION(imageaffine)
if ((zval_affine_elem = zend_hash_index_find(Z_ARRVAL_P(z_affine), i)) != NULL) {
switch (Z_TYPE_P(zval_affine_elem)) {
case IS_LONG:
affine[i] = Z_LVAL_P(zval_affine_elem);
affine[i] = (double) Z_LVAL_P(zval_affine_elem);
if (affine[i] < INT_MIN || affine[i] > INT_MAX) {
zend_argument_value_error(2, "element %i must be between %d and %d", i, INT_MIN, INT_MAX);
RETURN_THROWS();
Expand Down Expand Up @@ -4195,7 +4195,7 @@ PHP_FUNCTION(imageaffinematrixconcat)
if ((tmp = zend_hash_index_find(Z_ARRVAL_P(z_m1), i)) != NULL) {
switch (Z_TYPE_P(tmp)) {
case IS_LONG:
m1[i] = Z_LVAL_P(tmp);
m1[i] = (double) Z_LVAL_P(tmp);
break;
case IS_DOUBLE:
m1[i] = Z_DVAL_P(tmp);
Expand All @@ -4212,7 +4212,7 @@ PHP_FUNCTION(imageaffinematrixconcat)
if ((tmp = zend_hash_index_find(Z_ARRVAL_P(z_m2), i)) != NULL) {
switch (Z_TYPE_P(tmp)) {
case IS_LONG:
m2[i] = Z_LVAL_P(tmp);
m2[i] = (double) Z_LVAL_P(tmp);
break;
case IS_DOUBLE:
m2[i] = Z_DVAL_P(tmp);
Expand Down
10 changes: 5 additions & 5 deletions ext/gd/libgd/gd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,7 @@ void gdImageLine (gdImagePtr im, int x1, int y1, int x2, int y2, int color)
TBB: but watch out for /0! */
double ac = cos (atan2 (dy, dx));
if (ac != 0) {
wid = thick / ac;
wid = (int) (thick / ac);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that rounding would make sense here, but likely introduces a subtle BC break; as such casting to int is okay.

} else {
wid = 1;
}
Expand Down Expand Up @@ -1198,7 +1198,7 @@ TBB: but watch out for /0! */
TBB: but watch out for /0! */
double as = sin (atan2 (dy, dx));
if (as != 0) {
wid = thick / as;
wid = (int) (thick / as);
} else {
wid = 1;
}
Expand Down Expand Up @@ -1388,7 +1388,7 @@ void gdImageDashedLine (gdImagePtr im, int x1, int y1, int x2, int y2, int color
TBB: but watch out for /0! */
double as = sin(atan2(dy, dx));
if (as != 0) {
wid = thick / as;
wid = (int) (thick / as);
} else {
wid = 1;
}
Expand Down Expand Up @@ -1437,7 +1437,7 @@ void gdImageDashedLine (gdImagePtr im, int x1, int y1, int x2, int y2, int color
TBB: but watch out for /0! */
double as = sin (atan2 (dy, dx));
if (as != 0) {
wid = thick / as;
wid = (int) (thick / as);
} else {
wid = 1;
}
Expand Down Expand Up @@ -2827,7 +2827,7 @@ void gdImageFilledPolygon (gdImagePtr im, gdPointPtr p, int n, int c)
* same footprint.
*/
if (y >= y1 && y < y2) {
im->polyInts[ints++] = (float) ((y - y1) * (x2 - x1)) / (float) (y2 - y1) + 0.5 + x1;
im->polyInts[ints++] = (int) ((float) ((y - y1) * (x2 - x1)) / (float) (y2 - y1) + 0.5 + x1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is okay.

} else if (y == pmaxy && y == y2) {
im->polyInts[ints++] = x2;
}
Expand Down
4 changes: 2 additions & 2 deletions ext/gd/libgd/gd_avif.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ static int quality2Quantizer(int quality) {

float scaleFactor = (float) AVIF_QUANTIZER_WORST_QUALITY / (float) MAX_QUALITY;

return round(scaleFactor * (MAX_QUALITY - clampedQuality));
return (int) round(scaleFactor * (MAX_QUALITY - clampedQuality));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the float calculation is unnecessary here. The following should be good enough:

Suggested change
return (int) round(scaleFactor * (MAX_QUALITY - clampedQuality));
return clampedQuality * AVIF_QUANTIZER_WORST_QUALITY / MAX_QUALITY;

But changing this now would introduce a subtle BC break, so just casting to int appears to be sensible for now.

}

/*
Expand All @@ -103,7 +103,7 @@ static avifBool setEncoderTilesAndThreads(avifEncoder *encoder, avifRGBImage *rg

// The number of tiles in any dimension will always be a power of 2. We can only specify log(2)tiles.

tilesLog2 = floor(log2(tiles));
tilesLog2 = (int) floor(log2(tiles));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we even floor() here instead of casting to int right away:

Suggested change
tilesLog2 = (int) floor(log2(tiles));
tilesLog2 = (int) log2(tiles);

And given we're interested in the log2 of an integer, there are even options not requiring an FP instruction (especially since tiles <= 8). But okay.


// If the image's width is greater than the height, use more tile columns
// than tile rows to make the tile size close to a square.
Expand Down
3 changes: 1 addition & 2 deletions ext/gd/libgd/gd_interpolation.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2137,8 +2137,7 @@ gdImagePtr gdImageRotateInterpolated(const gdImagePtr src, const float angle, in
/* round to two decimals and keep the 100x multiplication to use it in the common square angles
case later. Keep the two decimal precisions so smaller rotation steps can be done, useful for
slow animations. */
const int angle_rounded = fmod((int) floorf(angle * 100), 360 * 100);

const int angle_rounded = (int) fmod((int) floorf(angle * 100), 360 * 100);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks seriously overengineered. Why are there even float operations, instead of

Suggested change
const int angle_rounded = (int) fmod((int) floorf(angle * 100), 360 * 100);
const int angle_rounded = ((int) (angle * 100)) % (360 * 100);

if (bgcolor < 0) {
return NULL;
}
Expand Down
2 changes: 1 addition & 1 deletion ext/gd/libgd/gd_webp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ void gdImageWebpCtx (gdImagePtr im, gdIOCtx * outfile, int quality)
if (quality >= gdWebpLossless) {
out_size = WebPEncodeLosslessRGBA(argb, gdImageSX(im), gdImageSY(im), gdImageSX(im) * 4, &out);
} else {
out_size = WebPEncodeRGBA(argb, gdImageSX(im), gdImageSY(im), gdImageSX(im) * 4, quality, &out);
out_size = WebPEncodeRGBA(argb, gdImageSX(im), gdImageSY(im), gdImageSX(im) * 4, (float) quality, &out);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The simple cast is fine here to suppress the warning. ext/gd catches values < -1 early, and for libgd, such values cause WebPEncodeRGBA() to fail.

}

if (out_size == 0) {
Expand Down
2 changes: 1 addition & 1 deletion ext/intl/collator/collator_convert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ zval* collator_convert_string_to_double( zval* str, zval *rv )
zval* num = collator_convert_string_to_number( str, rv );
if( Z_TYPE_P(num) == IS_LONG )
{
ZVAL_DOUBLE( num, Z_LVAL_P( num ) );
ZVAL_DOUBLE( num, (double) Z_LVAL_P( num ) );
}

return num;
Expand Down
2 changes: 1 addition & 1 deletion ext/intl/formatter/formatter_parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ PHP_FUNCTION( numfmt_parse )
case FORMAT_TYPE_INT64:
val64 = unum_parseInt64(FORMATTER_OBJECT(nfo), sstr, sstr_len, position_p, &INTL_DATA_ERROR_CODE(nfo));
if(val64 > ZEND_LONG_MAX || val64 < ZEND_LONG_MIN) {
RETVAL_DOUBLE(val64);
RETVAL_DOUBLE((double) val64);
} else {
RETVAL_LONG((zend_long)val64);
}
Expand Down
2 changes: 1 addition & 1 deletion ext/mbstring/mbstring.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3348,7 +3348,7 @@ try_next_encoding:;
}

for (size_t i = 0; i < length; i++) {
array[i].demerits *= array[i].multiplier;
array[i].demerits *= (uint64_t) array[i].multiplier;
}

return length;
Expand Down
2 changes: 1 addition & 1 deletion ext/opcache/zend_accelerator_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ ZEND_FUNCTION(opcache_get_configuration)
add_assoc_long(&directives, "opcache.jit_max_recursive_returns", JIT_G(max_recursive_returns));
add_assoc_long(&directives, "opcache.jit_max_root_traces", JIT_G(max_root_traces));
add_assoc_long(&directives, "opcache.jit_max_side_traces", JIT_G(max_side_traces));
add_assoc_long(&directives, "opcache.jit_prof_threshold", JIT_G(prof_threshold));
add_assoc_long(&directives, "opcache.jit_prof_threshold", (zend_long) JIT_G(prof_threshold));
add_assoc_long(&directives, "opcache.jit_max_trace_length", JIT_G(max_trace_length));
#endif

Expand Down
4 changes: 2 additions & 2 deletions ext/random/gammasection.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ static double gamma_max(double x, double y)

static void splitint64(uint64_t v, double *vhi, double *vlo)
{
*vhi = v >> 2;
*vlo = v & UINT64_C(0x3);
*vhi = (double) (v >> 2);
*vlo = (double) (v & UINT64_C(0x3));
}

static uint64_t ceilint(double a, double b, double g)
Expand Down
2 changes: 1 addition & 1 deletion ext/random/random.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ PHPAPI zend_long php_mt_rand_common(zend_long min, zend_long max)
/* This is an inlined version of the RAND_RANGE_BADSCALING macro that does not invoke UB when encountering
* (max - min) > ZEND_LONG_MAX.
*/
zend_ulong offset = (double) ( (double) max - min + 1.0) * (r / (PHP_MT_RAND_MAX + 1.0));
zend_ulong offset = (zend_ulong) (((double) max - min + 1.0) * (r / (PHP_MT_RAND_MAX + 1.0)));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old version appears to "over-cast", so I simplified. @TimWolla, @zeriyoshi, thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good!


return (zend_long) (offset + min);
}
Expand Down
2 changes: 1 addition & 1 deletion ext/random/randomizer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ PHP_METHOD(Random_Randomizer, getInt)
/* This is an inlined version of the RAND_RANGE_BADSCALING macro that does not invoke UB when encountering
* (max - min) > ZEND_LONG_MAX.
*/
zend_ulong offset = (double) ( (double) max - min + 1.0) * (r / (PHP_MT_RAND_MAX + 1.0));
zend_ulong offset = (zend_ulong) (((double) max - min + 1.0) * (r / (PHP_MT_RAND_MAX + 1.0)));

result = (zend_long) (offset + min);
} else {
Expand Down
2 changes: 1 addition & 1 deletion ext/soap/php_encoding.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,7 @@ static zval *to_zval_double(zval *ret, encodeTypePtr type, xmlNodePtr data)
whiteSpace_collapse(data->children->content);
switch (is_numeric_string((char*)data->children->content, strlen((char*)data->children->content), &lval, &dval, 0)) {
case IS_LONG:
ZVAL_DOUBLE(ret, lval);
ZVAL_DOUBLE(ret, (double) lval);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this cast fine, @nielsdos?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

break;
case IS_DOUBLE:
ZVAL_DOUBLE(ret, dval);
Expand Down
Loading