MSVC warnings around negating unsigned types

[nazar-pc]

Bug Report

MSVC doesn't like when unsigned types are negating. Not sure what other compilers think about it, but better fix them since it might have weird side effects.

Your environment

• Operating system: Windows Server 2019
• gcc/clang version: MSVC 19.29.30038.1

Issue description

This is de-duplicated list of C4146: unary minus operator applied to unsigned type, result still unsigned:

..\..\include\RTC/RTCP/ReceiverReport.hpp(79): warning C4146: unary minus operator applied to unsigned type, result still unsigned
..\..\include\RTC/RTCP/FeedbackRtpTransport.hpp(279): warning C4146: unary minus operator applied to unsigned type, result still unsigned
../../src/RTC/RTCP/Sdes.cpp(159): warning C4146: unary minus operator applied to unsigned type, result still unsigned
../../src/RTC/RTCP/FeedbackPsRpsi.cpp(53): warning C4146: unary minus operator applied to unsigned type, result still unsigned
../../src/RTC/RTCP/FeedbackPsVbcm.cpp(39): warning C4146: unary minus operator applied to unsigned type, result still unsigned
../../src/RTC/RTCP/FeedbackRtpTransport.cpp(308): warning C4146: unary minus operator applied to unsigned type, result still unsigned
../../src/RTC/RTCP/FeedbackRtpTransport.cpp(394): warning C4146: unary minus operator applied to unsigned type, result still unsigned

Here is the pipeline run with those complaints: https://github.com/nazar-pc/mediasoup/runs/3152962802?check_suite_focus=true

[ibc]

Thanks, will check tomorrow.

[ibc]

I didn't check it tomorrow

[ibc]

@nazar-pc we don't really know how to fix these warnings. Trying things like this:

diff --git a/worker/src/RTC/RTCP/Sdes.cpp b/worker/src/RTC/RTCP/Sdes.cpp
index 05ada2586..86b868df8 100644
--- a/worker/src/RTC/RTCP/Sdes.cpp
+++ b/worker/src/RTC/RTCP/Sdes.cpp
@@ -156,7 +156,7 @@ namespace RTC
 			}
 
 			// 32 bits padding.
-			size_t padding = (-offset) & 3;
+			size_t padding = static_cast<size_t>(-1 * offset) & 3;
 
 			for (size_t i{ 0 }; i < padding; ++i)
 			{

Can you please test this in Windows compiler?

[nazar-pc]

I think what it wants you to do is to cast unsigned integer to signed integer first and then negate it instead of negating an unsigned integer directly. It is weird that some compilers complain about it and some do not.

[ibc]

@nazar-pc AFAIU it's not possible to replicate these warnings using clang in macOS, right?

[nazar-pc]

Maybe with some parameters, but not with defaults for sure. Different compilers have different warnings enabled by default.

[ibc]

From Visual Studio 2022 docs: https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-2-c4146?view=msvc-170

This warning often occurs when you try to express the minimum int value, -2147483648, or the minimum long long value, -9223372036854775808. These values can't be written as -2147483648 or -9223372036854775808ll, respectively. The reason is because the compiler processes the expression in two stages: first, it parses the numeric value, then it applies the negation operator. For example, when the compiler parses -2147483648, it follows these steps:

1. The number 2147483648 is evaluated. Because it's greater than the maximum int value of 2147483647, but still fits in an unsigned int, the type of 2147483648 is unsigned int.
2. Unary minus is applied to the unsigned value, with an unsigned result, which also happens to be 2147483648.

So this is just a warning and we are fine. Closing...

[nazar-pc]

It is not fine to have warnings in the codebase IMHO. And there is A LOT of warnings under MSVC that look like potential issues and should be fixed.

[ibc]

😣😣😣😣😣😣