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 all commits
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 @@ -32,7 +32,7 @@ if "%THREAD_SAFE%" equ "0" set ADD_CONF=%ADD_CONF% --disable-zts
if "%INTRINSICS%" neq "" set ADD_CONF=%ADD_CONF% --enable-native-intrinsics=%INTRINSICS%
if "%ASAN%" equ "1" set ADD_CONF=%ADD_CONF% --enable-sanitizer --enable-debug-pack

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

cmd /c configure.bat ^
--enable-snapshot-build ^
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
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
Loading