Fix comparison and narrowing errors reported by GCC

[am11]

Contributes to: #22684

[franksinankaya]

I removed unused function warnings

[am11]

I removed unused function warnings

Is it in a separate branch?

I have made these changes on top of your gcc_compile branch (which has been slightly diverged from dotnet's master), built on GCC. The overall progress is around 12%. Next batch of errors come from gc.cpp

After that I cherry-picked my commits on top of dotnet/master, compiled with clang and pushed.

[franksinankaya]

I will rebase that branch

[am11]

OK, do you suggest that we leave out changes made in src/inc/internalunknownimpl.h in this PR?

[franksinankaya]

Here is the new branch: https://github.com/franksinankaya/coreclr/pull/new/gcc_compile_rebase

[AaronRobinsonMSFT]

This is not the same test anymore. Was the previous test not valid or unnecessary?

[am11]

Three compilers have different issues with the implementation.

For Clang & GCC, see: https://stackoverflow.com/a/24967341

clang had no issues with va_args, but warned on va_start: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior

gcc had no issue with va_start, but warned about promotion in va_args (implied that va_args's second argument should explicitly be double).

MSVC required more casts: float x = (float)doubleValue (note original version already has few of them).

This is not the same test anymore.

If according to C99 and C11 standards (which C++ include by reference), these promotions may take place under the wraps, then this change only makes the compilers' implicit behavior explicit, no?

[AaronRobinsonMSFT]

Was the previous test not valid or unnecessary?

Seems that the previous test wasn't really valid as written. I didn't know the promotion rules regarding varargs - thanks. This change makes sense then as I agree making the argument type explicit is a laudable goal. Someone from @dotnet/jit-contrib should review this test change as well.

[franksinankaya]

It is on master branch now. I need to rebase that branch. I will let you know.

…

________________________________ From: Adeel Mujahid <notifications@github.com> Sent: Sunday, February 24, 2019 4:50:41 PM To: dotnet/coreclr Cc: Sinan Kaya; Mention Subject: Re: [dotnet/coreclr] Fix comparison and narrowing errors reported by GCC (#22821) I removed unused function warnings Is it in a separate branch? I have made these changes on top of your gcc_compile branch (which has been slightly diverged from dotnet's master), built on GCC. The overall progress is around 12%. Next batch of errors come from gc.cpp After that I cherry-picked my commits on top of dotnet/master, compiled with clang and pushed. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fcoreclr%2Fpull%2F22821%23issuecomment-466820805&data=02%7C01%7Csinan.kaya%40microsoft.com%7Cbf48d10a3188479dce4808d69aa21ee2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636866418427076276&sdata=r1tNyVIBec7iW%2F%2B3mWqhPBrqXM2uIX5BXcXZVp7aics%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAn31pvTUfRYs1atLeACAU_Le-LDZsssYks5vQwkxgaJpZM4bOw6Z&data=02%7C01%7Csinan.kaya%40microsoft.com%7Cbf48d10a3188479dce4808d69aa21ee2%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636866418427086276&sdata=U8vK2y%2B4KJLtLT324t%2BZ6HXYQ3eVPk%2B77rC8Fv7v4Dc%3D&reserved=0>.

[am11]

For the record, I am now pressing Ready for review button. 😸

[am11]

Thanks for all your reviews, tips, tricks and traps! (looking for more)

[am11]

test Ubuntu x64 Checked CoreFX Tests
test OSX10.12 x64 Checked Innerloop Build and Test

[franksinankaya]

This looks good to me. Any other comments?

[janvorli]

LGTM, thank you!