Fix clang-cl / g++ compilation

[peter-urban]

Compiling the test executable using clang-cl on windows causes segfaults during the execution. It took me a while to understand that on windows compilation happens in c++ So I tested compilation as c++ lib on linux (g++ -x c++ ) and got the same segfaults.

Turns out the problem is in check_functions.c / "assert_equal" The function specifies return type "int" even though it doesn't return anything (senseful)

Specifying 'void' fixes the segfaults.

I added the tests for the compilation with clang-cl and g++, and since I didn't know better created special Makefiles which set CPP to "clang-cl /TP" or "g++ -x c++"

Let me know if you know of a more elegant solution for these makefiles or if you want me to remove the extra tests/makefiles from this pull request entirely

Cheers and thanks for this amazing project

[efiring]

Thank you for finding and tracking that down. I don't see any problem with keeping the makefiles and tests, but I defer to @ocefpaf.

[efiring]

Looking at this again, I'm wondering whether the 3 makefiles (one old, two new) can be condensed back down to one, given that they differ by a single line. I think this would require using a new environment variable to act as a switch, changing the compilation command; and this would require additional ugly blocks in the makefile if nmake compatibility is to be retained. So, it might be a cure worse than the disease.

[DocOtak]

In gnu make (not sure about other implementations), you can use a conditional assignment ?= for the CPP variable for a default, then override in envars in CI. I think some of these vars (e.g. CPP) are special in that make has a default for them and they should not necessarily be defined in the makefile itself.

[peter-urban]

@DocOtak thanks for the suggestion, unfortunately nmake does not reckognize the ?= logic I tried around for a while and managed to put the COMPILER conditional into the nmake/make conditional block that was used to include tools.gcc for make.

So now it is one makefile (though not a specifically beautifull one)

[efiring]

I'm concerned that I have contributed to opening a can of worms. Before this PR, we had one moderately ugly makefile, with all the ugliness arising from trying to accommodate both make and nmake. In the original version of the PR, the makefile ugliness stayed the same but was duplicated twice, with the two new makefiles. Given that nmake needs to be supported only for the Microsoft compiler, maybe the solution is to have 2 simple and clean makefiles, one for nmake, and the other for make. That would eliminate the original hack. I presume the non-MS makefile could then be restored to a simple and standard form (with the CC environment variable handling all compiler specifications?), and the CI job(s) might also end up back to something like their prior relatively simple states. Is this correct? I don't want to launch another goose (or worm) chase.

The most important part of the PR is the one-line fix to the actual test code. Beyond that, we want to make future understanding, maintenance, and use of the project easier, not harder.

[efiring]

OK, I think we are either very close, or at a reasonable stopping point. It looks to me like there is one misconception that has been inherited from the original nmake addition: #39. It introduced the use of CPP for the compiler; but CPP is supposed to be the C preprocessor command. Probably what was meant was CXX, which is for invoking the C++ compiler, as compared to CC for the C compiler. These standards in make came from the earlier era when C and C++ compilers were more distinct. Given that the library is fundamentally a C library--even when compiled with C++, it uses 'extern "C"' for all exports--I think that all instances of "CPP" should be replaced with "CC". We don't need or use the C preprocessor for any separate step, so we don't need to even have a CPP variable.

Beyond that, it is not clear to me why the additional "COMPILER" variable is even needed; wouldn't everything work with the assumption that CC, if it is not the make or nmake default, is defined in the shell before invoking make or nmake? In other words, it doesn't need to be in TOOLS at all?

[peter-urban]

@efiring yes this turned out to be quite a worm can with intense CI debugging... Now I understand why I am not working with Makefiles ;-D

Let me guide you through the proposed solution:

1. Turns out that variables can be overwritten by calling (n)make CC=clang-cl. No need for /E and ?= etc.
2. As you suggested, I removed the CPP variable and used a CC variable only.
3. I find a variable called "CC=cl /TP" inconsistent. The CC environment variable is typically associated with an executable/command without flags. I thus moved the /TP flag to a new EXTRA_FLAGS variable
4. I think the /TP option (compiling in c++ on windows) and the make/nmake confusion need explaining, so I added comments to the Makefile
5. There was yet another inconsistency in the make file:
   • the library: uses $(LFLAGS) which includes $(LRELEASE)
   • the program: uses $(CRELEASE) even though there is also a $(CFLAGS) variable which includes $(CRELEASE)
   • I replaced $(CRELEASE) by $(CFLAGS) in the Makefile call for consistency

[efiring]

@peter-urban Thank you for your persistence and skill in wrangling the worms back into the makefile can after finding and fixing the original bug.