Nemirtingas / System

GNU General Public License v3.0
0 stars 1 forks source link

Missing standard include `cstdint` fails the Github Workflow #8

Closed otavepto closed 9 months ago

otavepto commented 9 months ago

The usage of std::uintxxx in places, for example, like https://github.com/Nemirtingas/System/blob/0969ca178a1002ca75412b75011d52a3f85e21a4/include/System/Encoding.hpp#L59-L60

Requires #include <cstdint>, but the lack of it fails the Github Workflow, here's an example: https://github.com/otavepto/gbe_fork/actions/runs/8127429546/job/22212242106#step:7:5313

This is the build command:

cd /home/runner/work/gbe_fork/gbe_fork/build/deps/linux/ingame_overlay/build64/deps/System && /usr/bin/clang++  -I/home/runner/work/gbe_fork/gbe_fork/build/deps/linux/ingame_overlay/deps/System/include -I/home/runner/work/gbe_fork/gbe_fork/build/deps/linux/ingame_overlay/deps/System/deps/utfcpp/include -O3 -DNDEBUG -std=c++14 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -MD -MT deps/System/CMakeFiles/system.dir/src/Library.cpp.o -MF CMakeFiles/system.dir/src/Library.cpp.o.d -o CMakeFiles/system.dir/src/Library.cpp.o -c /home/runner/work/gbe_fork/gbe_fork/build/deps/linux/ingame_overlay/deps/System/src/Library.cpp

And this is the error underneath it:

In file included from /home/runner/work/gbe_fork/gbe_fork/build/deps/linux/ingame_overlay/deps/System/src/Library.cpp:21:
/home/runner/work/gbe_fork/gbe_fork/build/deps/linux/ingame_overlay/deps/System/include/System/Encoding.hpp:59:32: error: no type named 'uint8_t' in namespace 'std'
inline std::string Encode(std::uint8_t const* data, std::size_t len, bool padding)
                          ~~~~~^

no type named 'uint8_t' in namespace 'std'


To workaround this, I'm adding this to the command line when invoking CMAKE:

-DCMAKE_CXX_FLAGS_RELEASE='-include cstdint -include cinttypes'

This forces clang (and gcc) to put #include "file" at the top of each file, the docs for this compiler flag (-include) could be found here: https://gcc.gnu.org/onlinedocs/gcc/Preprocessor-Options.html

Strangely enough, on my machine I'm running WSL on Windows 10 with ubuntu 22, and I have a dedicated ubuntu VM, and none of them triggered this problem.

Nemirtingas commented 9 months ago

Hi, You should just add #include <cstdint> under the #include <string> directive :)

otavepto commented 9 months ago

Yeah I just didn't want to edit the code, so I thought I might bring this to your attention. It's definitely unusual since most x86 compilers will add some default includes, Feel free to close this one, just wanted to let you know.

Nemirtingas commented 9 months ago

I highly doubt that modern compilers add includes on their own. That would completly break custom implementations. MSVC might, cause Microsoft always keeps doing stuff that break cross-platform. They call theses stuff "Extensions"... So when you create something, you compile it on MSVC, it works fine, go on GCC, it doesn't compile ;).

This is an issue not matter what. The correct fix to always add this include. I will push a fix.

Nemirtingas commented 9 months ago

https://github.com/Nemirtingas/System/commit/4a9c7530dda22fbb9abcb4807b2cb0f14483d12a

otavepto commented 9 months ago

Thank you