Multi compiler analysis

Inter-compiler programming mindset is a very conscious attempt which I strive for implementing and achieving practical fluidity with. The reason, being simple enough, is equally difficult to articulate, to uninitiated. If you were a traveler, and with good sense of appreciation, you would want to be able to travel across any Earthly place. If you were a Physicist, you would want to see a symmetry in space, that your experimental results should not depend on the Country where you perform them. Physicists call it homogeneity which results in a law called "Conservation of Momentum". If you still are not convinced, I politely request you to spend the energy of your un-ciritcal mind elsewhere.

That said, one of the advantages of writing such code is ability to cater multiple platforms (Linux, Mac, Windows, Playstation, XBox, and mobiles). Yeah there already are tools and applications doing the same thing, which is part of the charm, especially because of the concept of "cross-over". If you were a comics fan, you can appreciate Batman-Superman cross-over comics and before you know it, Justice League is formed. The idea is simple, but implementation is equally difficult, to create solid enough context where each protagonist/antagonist has un-compromising role to play. Not for weak hearted, to digest of course. In the world of programming, this provides a playground for serious debates, discussions, magic, and fun whilst enhancing the scrutiny and adherence to the principles. Even with couple of years of experience with multiplatform software development, I believe we have reached the stage where we should be concerned about maintenance rather than finding new uncertain and rather more dangerous terrains and the purpose of this blog-post is to elicit just that.

Chiefly there are three major compilers GCC (my personal favorite), Clang, and MSVC (yeah that one) that I shall be focusing upon for my C++ desires. Karma is an attempt to reconcile the untended differences which crept into them during the course of software evolution. Humans write compilers and softwares. And if you can digest that, there should be no doubt on why inter-compiler difference should be addressed with usual rigor and vigor.

We start with the analysis of commit 85f6630. On MacOS Monterey, the code compiles with no warnings. The compiler is Clang/LLVM (AppleClang 13.1.6.13160021). But the same code is coming up with warnings when compiled by GCC 11.2.0 on Ubuntu 22.04, like so

/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/ImGui/imgui_widgets.cpp: In function ‘bool ImGui::CollapsingHeader(const char*, bool*, ImGuiTreeNodeFlags)’:
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/ImGui/imgui_widgets.cpp:5907:54: warning: bitwise operation between different enumeration types ‘ImGuiTreeNodeFlags_’ and ‘ImGuiTreeNodeFlagsPrivate_’ is deprecated [-Wdeprecated-enum-enum-conversion]
 5907 |         flags |= ImGuiTreeNodeFlags_AllowItemOverlap | ImGuiTreeNodeFlags_ClipLabelForTrailingButton;
      |                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzread.c: In function ‘gz_load’:
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzread.c:35:15: warning: implicit declaration of function ‘read’; did you mean ‘fread’? [-Wimplicit-function-declaration]
   35 |         ret = read(state->fd, buf + *have, get);
      |               ^~~~
      |               fread
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzread.c: In function ‘gzclose_r’:
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzread.c:649:11: warning: implicit declaration of function ‘close’; did you mean ‘pclose’? [-Wimplicit-function-declaration]
  649 |     ret = close(state->fd);
      |           ^~~~~
      |           pclose

and finally

/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzlib.c: In function ‘gz_open’:
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzlib.c:14:17: warning: implicit declaration of function ‘lseek’; did you mean ‘fseek’? [-Wimplicit-function-declaration]
   14 | #  define LSEEK lseek
      |                 ^~~~~
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzlib.c:252:9: note: in expansion of macro ‘LSEEK’
  252 |         LSEEK(state->fd, 0, SEEK_END);  /* so gzoffset() is correct */
      |         ^~~~~
[174/238 1.6/sec] Building C object Karma/vendor/assimp/code/CMakeFiles/AssImp.dir/__/contrib/zlib/gzwrite.c.o
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzwrite.c: In function ‘gz_comp’:
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzwrite.c:89:20: warning: implicit declaration of function ‘write’; did you mean ‘fwrite’? [-Wimplicit-function-declaration]
   89 |             writ = write(state->fd, strm->next_in, put);
      |                    ^~~~~
      |                    fwrite
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzwrite.c: In function ‘gzclose_w’:
/home/the_cowboy/EngineDev/KarmaEngine/Karma/vendor/assimp/contrib/zlib/gzwrite.c:664:9: warning: implicit declaration of function ‘close’; did you mean ‘pclose’? [-Wimplicit-function-declaration]
  664 |     if (close(state->fd) == -1)
      |         ^~~~~
      |         pclose

The first question that strikes me is not why these warnings are not visible to Clang, but what we are going to do when we know they exist from GCC POV. A very nice thing is that the process of compilation goes through either ways (we do get the intended running binaries). So now it becomes a question of semantics.

The first GCC warning is concerned with the bitwise operation over different enumeration types. The code is as follows (find it here)

if (p_open)
        flags |= ImGuiTreeNodeFlags_AllowItemOverlap | ImGuiTreeNodeFlags_ClipLabelForTrailingButton;

meaning it was valid before and now it is deprecated. That conforms with Clang in the sense of starting point (when both allowed the operation) but dynamically the compilers are differing for the same language! Now clearly empires have been uprooted and hell been set loose (remember Sodom and Gomorrah) for compilers not conforming naturally or to God's wish (if you are into Bible and whatnot). The point is not that they are differing, the main thing is it should be clear how they are differing with their respective POVs.

Now it is time to make a judgement since I can't know the internal dynamics that led GCC to deprecate the inter-enum bitwise operations. But the judgement should have firm basis and not some deluded sense of power abuse. Not only I have to demonstrate how I reached to a particular judgement, but also show enough compassion to stand up responsibly should the time demand because the decision I make may or may not affect the upcoming C++ standards in some direct or indirect way. I may write a note on the interplay of compilers and languages and how that affects programming ethos and ultimately evolution of a community or Society itself in the big picture.

If I understand correctly, there is no scope of error if the enums have different values assigned explicitly (which be the case at hand). And if that is the true sense of the enum usage, then I don't see the reason for depreciation (unless experienced opinions have a bunny in their hat). And they do! One of the Google search results indicate the possibility of overlapping enum values and thus the warning is a safety check (if I got it right). Now assuming certain level of rationality of a decent programmer, I'd consider this his typo rather than supplying spoon-feeding treatment by generating, now, redundant warning! So Apple (Clang) takes the biscuit (a sentence I haven't imagined writing).

If GCC has some ulterior motive other than spoon-feeding then I don't understand and would stick with Apple Clang's way by disabling the warning. While we are on the topic, let me dig little deeper to drench myself with the pedantic thirst of quantifying language dialects and compiler workings because "I am not understanding what I am seeing" (IANUWIAS). The official GCC documentation (written here) says

Disable the warning about the case when the usual arithmetic conversions are applied on operands where one is of enumeration type and the other is of a different enumeration type. This conversion was deprecated in C++20. For example:

enum E1 { e };
enum E2 { f };
int k = f - e;

-Wdeprecated-enum-enum-conversion is enabled by default with -std=c++20. In pre-C++20 dialects, this warning can be enabled by -Wenum-conversion. 

So it seems GCC is compliant with C++ standards (and Apple Clang is not!). On compiling the same code with C++11 using GCC, the warning does not appear, as noted by GCC who even take a step futher by providing a tool for backwards compatibility with the warning scheme. The price to pay, literally, is mentioned brazenly here. Since I am broke and can't afford the standards, the take away I draw is that GCC is swift with the C++ standards when compared with Apple Clang.

Moving on, we focus on next series of warnings originating from zlib. Their unifying factor is the implicit declaration of the following functions read, write, close, and lseek. Few things to note

1. These warnings don't appear in Apple Clang.
2. Implicit declaration is essentially compiler's assumption when complete relevant information is not found.
3. Zlib is a C library.

Had a light read of post which indicates that these warnings better be taken as errors because compiler has no clue about the function prototype and thus makes some implicit assumptions. Now I do remember Apple Clang spitting out errors for zlib and the fix was to include unistd header (65f821a and cc2b36c). So basically it was my ignorance to not completely understand the role of unistd which contains declarations for the above mentioned functions. Hence the exclusion from Linux platform explains the warnings.

But hold on, warnings? These are actual errors in my books. Making implicit assumptions about function prototypes is stupid zone (as demonstrated in the above linked blog-post). Apple Clang again takes the biscuit. The directive is to treat such warnings as blasphemous errors.

Decisions... the CMake build configurations now has two new entries for compiler flags (for both C and C++) like so

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-deprecated-enum-enum-conversion") # inter-enum bitwise operations
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror-implicit-function-declaration") # treat implicit declarations as errors

Finally fix the error by relevant inclusion 8f32272.

Thus our contention about inter-compiler framework yielding code stands corrected. Not only we can find our own mistakes but also gather enough courage to make solid assertions about the things we desire. Madonna rules! (https://www.youtube.com/watch?v=XS088Opj9o0)

Now after addressing GCC and (Apple) Clang, let me focus my attention towards MSVC, a different beast altogether, and try to understand the warnings thus generated for the same code. I have some reservations but let that not interfere with my critical analysis power! So, let me list the warnings

8>C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\um\winbase.h(9531,5): warning C5105: macro expansion producing 'defined' has undefined behavior
9>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\glslang\glslang\MachineIndependent\Constant.cpp(245,114): warning C4146: unary minus operator applied to unsigned type, result still unsigned
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\code\Common\FileSystemFilter.h(303,27): warning C4244: 'initializing': conversion from '__int64' to 'int', possible loss of data (compiling source file C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\code\Common\BaseImporter.cpp)
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\code\Common\DefaultIOSystem.cpp(125,14): warning C4996: '_wfopen': This function or variable may be unsafe. Consider using _wfopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\code\Common\DefaultLogger.cpp(402,11): warning C4996: 'strcat': This function or variable may be unsafe. Consider using strcat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\code\Common\ImporterRegistry.cpp(217,31): warning C4996: 'getenv': This function or variable may be unsafe. Consider using _dupenv_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\code\Common\ZipArchiveIOSystem.cpp(128,5): warning C4996: 'strncpy': This function or variable may be unsafe. Consider using strncpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\code\PostProcessing\ValidateDataStructure.cpp(83,22): warning C4996: 'vsprintf': This function or variable may be unsafe. Consider using vsprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\code\AssetLib\FBX\FBXConverter.cpp(2154,37): warning C4244: 'initializing': conversion from 'double' to 'float', possible loss of data
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\unzip\ioapi.c(171,9): warning C4996: '_snprintf': This function or variable may be unsafe. Consider using _snprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzlib.c(193,15): warning C4996: 'wcstombs': This function or variable may be unsafe. Consider using wcstombs_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzlib.c(208,13): warning C4996: 'wcstombs': This function or variable may be unsafe. Consider using wcstombs_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzlib.c(243,20): warning C4996: '_wopen': This function or variable may be unsafe. Consider using _wsopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzlib.c(245,9): warning C4996: 'open': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _open. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzread.c(35,15): warning C4996: 'read': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _read. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzread.c(41,34): warning C4996: 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzread.c(649,11): warning C4996: 'close': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _close. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzwrite.c(89,20): warning C4996: 'write': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _write. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzwrite.c(91,42): warning C4996: 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzwrite.c(110,24): warning C4996: 'write': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _write. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzwrite.c(112,46): warning C4996: 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
10>C:\Users\the_cowboy\Misc\KarmaEngine\Karma\vendor\assimp\contrib\zlib\gzwrite.c(664,9): warning C4996: 'close': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _close. See online help for details.

So, the first warning is bit strange and incomprehensible to me. Seems to be originating from

#if MICROSOFT_WINDOWS_WINBASE_H_DEFINE_INTERLOCKED_CPLUSPLUS_OVERLOADS  /* { */

and I don't see any #endif closing clause. May or maynot be related and I can't check because of Microsoft's infinite wisdom. Gonna suppress it if I want to stick to my senses. Also read this. Also I updated Microsoft's SDK and have no inclination to figure out if it was the option or updation which got rid of the warning!

Moving on, next warning is concerned with unary operator (-) applied to unsigned type and result being unsigned still! Since I am out of my practical knowledge, I shall make certain assumptions based on my interaction with MSVC and couple of blog-posts (this and this).

It seems that the rvalue in hexadecimal form is unsigned datatype, for instance

somedatatype varD = -0xFFFFFFFB;

spits a warning that unary operator applied to unsigned type (irrespective of what the type of varD is). On the other hand

int varB = -5;

is alright. Let me demonstrate some more gymnastics

uint32_t varC = -1

has the value 4294967296 - 1 because of modulo effect.

Unsigned integer overflow

What happens if we try to store the number 280 (which requires 9 bits to represent) in a 1-byte (8-bit) unsigned integer? The answer is overflow.
If an unsigned value is out of range, it is divided by one greater than the largest number of the type, and only the remainder kept.

The number 280 is too big to fit in our 1-byte range of 0 to 255. 1 greater than the largest number of the type is 256. Therefore, we divide 280 by 256, getting 1 remainder 24. The remainder of 24 is what is stored.

Here’s another way to think about the same thing. Any number bigger than the largest number representable by the type simply “wraps around” (sometimes called “modulo wrapping”). 255 is in range of a 1-byte integer, so 255 is fine. 256, however, is outside the range, so it wraps around to the value 0. 257 wraps around to the value 1. 280 wraps around to the value 24.

If I want to assign the negation of hexadecimal rvalue (making the negative of value it self and not set/unset bits), then the following code should help

int varD = 0 - 0xFFFFFFFB;

the value thus assigned is 5 (note that 0xFFFFFFFB is 2's complement for representing -5 value). So we need be careful about and differentiate what bit-gymnastics we are performing (along with unary operators) and what the value is.

So if you really wanna do finky-slinky bitwise operations with unary operators and also don't want to give up their binary role, then use the trick of leading 0 (mentioned above). Here, in my judgement, MSVC takes the biscuit given I don't know what other compilers are thinking!

But we are not really meaning (-) as binary operator. In the code (linked here)

case EbtInt64:
                if (rightUnionArray[i] == 0ll)
                    newConstArray[i].setI64Const(0x7FFFFFFFFFFFFFFFll);
                else if (rightUnionArray[i].getI64Const() == -1 && leftUnionArray[i].getI64Const() == (long long)-0x8000000000000000ll)
                    newConstArray[i].setI64Const((long long)-0x8000000000000000ll);
                else
                    newConstArray[i].setI64Const(leftUnionArray[i].getI64Const() / rightUnionArray[i].getI64Const());
                break;

it seems that they want a platform independent way of estimating the constants. Then bit gymnastics is justified (for GCC and Clang). But, since given the compiler is MSVC we can't really be sure, so let me delve little deeper. Based on the discussion, at the source, it seems MSVC is really inconsistent piece of software. I suspect it be fool's errand to reason with "them" (yeah, you compiler writers!!). Here is another critic from "experienced" coder (link) whose writing is legit and sense of sarcasm is highly justified. If MSVC is gonna be so inconsistent that it forgets what it evaluated in few (bushel?) of lines before, there is no use finding sense, except just let them "evolve" with sarcasm!!

So it is clear, if the coder likes being spoon fed, then MSVC is for you, literally. For rest of the coders (which I assume be huge chunk), I believe you really know what is being done! Hence GCC and Clang are apt for rationality! I am leaving the warning C4146 as it is (a symbol of shame).

Moving on, next on the list is warning C4244 which points the loss of data on type conversion. The culprit seems to be the typo in code

int remaining = std::distance(in.end(), it);

which should be declared as long long local variable instead. Other compilers didn't catch this because they are not using MSVC's xutility tool and thus the return type seems to be matching the initially written type (maybe implicit cast, need to check). But we are going to change the type of said variable (since we are in surplus in memory and variable is local). More in the said category is this

const float roughness = 1.0 - (sqrt(ShininessExponent) / 10.0);

Need to find what other compilers are treating rvalue as, but MSVC is certainly seeing double. So casting that.

Next, we have a cluster of C4996 warnings. Seems like a library side work. Created a relevant thread. Gonna subscribe to that for future course of action. Meanwhile, I shall suppress the warnings for MSVC using the defines like so

if(MSVC)
	add_compile_definitions(_CRT_SECURE_NO_WARNINGS _CRT_NONSTDC_NO_DEPRECATE)
endif()

Need to do research why such functions are deprecated only for MSVC!