Improve cmake quality

[rawbby]

• added cmake compile options
• added cmake compile warnings
• raised cmake version to 3.5
• rewritten cmake code to be more idiomatic
• added files manually to improve IDE functionalities

[rawbby]

The cmake works fine on my machine. (win 10 - 64bit - MSVC 2019)
The code should also work on other platforms but I can not confirm.
Maybe it would be wise to create a build job that uses cmake to build flecs and its samples. (for each platform)

[SanderMertens]

The cmake file looks clean, but there is a significant version bump (2.8 -> 3.16). I would rather not require a newer cmake version if it is not strictly required.

Did a quick check on the cmake version on my mac and a few linux VMs:
macOS: 3.13
Ubuntu Xenial: 3.5.1
Ubuntu Bionic: 3.10.2
Ubuntu Focal: 3.16.2

[rawbby]

3.5.1 is a really old version of cmake in my oppinion.
3.12 is a realistic (modern) version of cmake that makes more sense to me.

The code should work with lower versions of cmake out of the box, but most projects set a higher cmake version to improve user experience.
Newer versions are more performant and you have more freedom for new features.
There are also some behavioral changes I am not sure about.
@see policies

[rawbby]

I am not sure whats the problem with apple clang :-(
@sander do you have any idea what special compiler options need to be specified for clang apple?

[codylico]

(outsider comment)
After skimming the logs, it looks like broken flecs::vectors (i.e. not a problem with Apple Clang). I may have half of a fix hiding somewhere; let me check.

[codylico]

@rawbby After a more thorough read of the logs, it seems like you may need to enable C++ 2011 somewhere in the build. I haven't checked if you have or not, though.

@SanderMertens Correct me if I am wrong, but I think the C++ API is intended to target the 2011 version.

[codylico]

My half of a fix is outdated. Sorry for the intrusion.

However, there seems to be a missing #include <initializer_list> for the C++ section of include/flecs/private/vector.h. I'm not sure that would take care of the rest of the issues. Apple Clang may actually have some problems with handling types in macros, but now I'm just guessing.

[SanderMertens]

3.5.1 is a really old version of cmake in my oppinion.
3.12 is a realistic (modern) version of cmake that makes more sense to me.

Agreed, Xenial will reach end of life this April, so I'm fine with a version >3.5 if it brings real advantages. I don't care so much about the code of the Flecs cmake file as it's just a write once thing, but if there are real benefits for cmake users I'd be happy to consider.

The default on Bionic seems to be 3.10.2 though, and it won't reach end of life until April 2023, so I don't think we should aim higher than 3.10.2 for now. Bionic will still be in use in many places, and upgrading cmake on Linux is super annoying: https://askubuntu.com/questions/355565/how-do-i-install-the-latest-version-of-cmake-from-the-command-line

Can you list what the improvements are for users would be of upgrading to a newer version?

My guess is that the compiler errors are caused because you're either not targeting (at least) C++11 or that you're missing the -D_XOPEN_SOURCE=600. For reference, on clang I use these compiler options when doing a strict build (all warnings enabled):

clang -fPIC -fvisibility=hidden -fno-stack-protector -g -O0 -std=c99 -D_XOPEN_SOURCE=600 -Wall -W 
-Wextra -Werror -Wshadow -Wconversion -Wwrite-strings -Wredundant-decls -pedantic -Wunused-parameter 
-Wsign-compare -Wcast-align -Wparentheses -Wsequence-point -Wpointer-arith -Wredundant-decls 
-Wdisabled-optimization -Wstrict-prototypes -Wnested-externs -Wmissing-declarations -Dflecs_EXPORTS 

A regular build (without all warnings enabled) looks like this:

clang -fPIC -fvisibility=hidden -fno-stack-protector -g -O0 -std=c99 -D_XOPEN_SOURCE=600 -Wall -W 
-Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Dflecs_EXPORTS 

[rawbby]

@codylico was right. There was a header file missing in vector.h but that was not the problem.
It is suspicious that the build only fails on mac os. Therefor I am relatively sure the problem is a compiler flag that needs to be set.
As I am not familiar with apple clang I dont know how to make the build working.

[codylico]

So I see things like the below, which made me guess that for some reason (compiler options, etc.) Apple Clang isn't handling the macros properly. I have no idea how to fix, though.

/Users/runner/work/flecs/flecs/include/flecs/private/vector.h:483:45: error: 'T' does not refer to a value
        T* elem = ecs_vector_add(&m_vector, T);
                                            ^
/Users/runner/work/flecs/flecs/include/flecs/private/vector.h:436:20: note: declared here
template <typename T>
                   ^

/Users/runner/work/flecs/flecs/include/flecs/private/vector.h:488:41: error: 'T' does not refer to a value
        return ecs_vector_get(m_vector, T, index);
                                        ^
/Users/runner/work/flecs/flecs/include/flecs/private/vector.h:436:20: note: declared here
template <typename T>
                   ^

[rawbby]

@SanderMertens can you confirm that building the c++ examples worked for you on a mac os machine?

[SanderMertens]

@SanderMertens can you confirm that building the c++ examples worked for you on a mac os machine?

All the clang-warnings CI tests are passing. They're not executed on macOS, but the C++ parser is the same. I just tried it with clang (v11.0.3) on my macOS machine and it also compiles.

These are the compiler flags that bake uses:

clang++ -fPIC -fvisibility=hidden -fno-stack-protector -g -O0 -std=c++0x -Wall -W 
-Wextra -Wno-unused-parameter -Wno-missing-field-initializers
-Dinheritance_EXPORTS -I./include -I /Users/sandermertens/bake/include

[rawbby]

@SanderMertens are there any changes you want to be made? From my side the code is fine for now :)

[SanderMertens]

Looks good! Thanks a lot :)

[SpaceIm]

Sorry, but I suggest to revert several modifications which have lead to lot of issues while trying to add a recipe of flecs in conan package manager:

• please do not force PIC for static lib (add an option to disable/enable this flag if you want)
• the worst: never reset incoming compiler flags, trust your consumers.
• CMake min version is too high for no good reason.
• level of warnings is too high (provide a switch for package managers or even simple consumer, we don't care about warnings, it's for core developers of the library)