Hi! I’m cross-posting this here from an AnswerHub post (https://answers.unrealengine.com/que…nsistency.html) at the suggestion of a commenter there. Hoping to get some input from folks at Epic. [HR][/HR]
I’d like to discuss signed/unsigned comparison warnings with the C++ compiler. Looking at the toolchain .cs files, looks like there’s been some internal debate. I would really like to get this out on the table, get this settled by Epic folks, and get some consistency in the compiler settings, particularly in cross-platform environments.
First, I’ll start with “the problem” – that is, why I’m bothering you about this in the first place. Our development team works primarily in Linux, but we support both Linux and Windows with our UE4-based software. On Linux, Clang doesn’t seem to care if we do checks between signed and unsigned types. On Windows, VC++ does care… but only sometimes. This adds overhead when Linux devs submit their feature branch pull requests, as we have to find/fix all of the sign issues.
I figured a simple “fix” to make things more consistent would be to enable sign checking on Clang. But when I turn this on, I get 100 build errors building the first 17 files, and the compile aborts. Unexpectedly, not all of the errors were in Linux-specific code.
I started digging into it, and this is what I found to be the current situation:
- On Windows, WindowsPlatformCompilerSetup.h disables warning C4389. This apparently prevents scenarios like Stats2.h, line 1313, which assigns a bool based on a comparison between a uint64 and an int32, from generating a warning.
- However, nothing Windows-side disables warning C4018, which generates warnings in a situation like [FONT=courier new]for(uint32 i=0; i < Arr.Num(); ++i).
- Note that for the life of me, I can’t seem to fathom the difference between the two above warnings. In two cases of comparing a signed and unsigned int (one in a for loop, one in a bool assignment), different warnings are generated.
- VCToolChain.cs passes “-Wno-sign-compare” to Clang when compiling with Clang on Windows (that is, when Target.WindowsPlatform.Compiler == WindowsCompiler.Clang; I don’t know for sure if this is making a Windows build with Clang, or if this is for cross-platform compiling), turning off sign warnings.
- On Linux, apparently Clang doesn’t generate sign warning by default. Nothing currently enables it, resulting in a notable difference between Linux and Windows error checking.
- LinuxToolChain.cs uses “-Wno-sign-compare” with GCC (not the default path) to disable sign warnings.
- On Mac, MacToolChain.cs at one time added “-Wsign-compare” to its compiler, enabling sign warnings. The comment reads “// fed up of not seeing the signed/unsigned warnings we get on Windows - lets enable them here too.” This is a relatable sentiment. However, the line is presently (as of 4.21) commented out, presumably defaulting back to no sign checks.
It seems to me that most of UE4 is geared toward suppressing the signed/unsigned comparison warnings.
Personally, I would argue the warnings should be enabled, and the issues (carefully) fixed. Comparison between signed and unsigned types is easy to do accidentally (as our team has proven), and can cause subtle and hard to find/fix bugs. If the developers at Epic would agree to this point, I would love for an issue to be created, the warnings to be enabled on on all platforms and compilers, and the resulting issues fixed (or suppressed in special cases where needed). In fact, I would be happy to consider doing this work and submitting it as a pull request.
If, however, Epic feels that the warnings are generally unnecessary and bothersome (as seems to be the case heretofore), I would like to see both of the Windows warnings disabled for the sake of consistency, particularly between platforms, and let the chips fall where they may. 
If anyone has input on the matter, I would love to hear opinions or arguments in either direction.
Thanks for your time and attention!
