nmwsharp / polyscope

A C++ & Python viewer for 3D data like meshes and point clouds
https://polyscope.run
MIT License
1.8k stars 196 forks source link

Issues building on GCC 13 and modern Clang (15+) #257

Closed berendbaas closed 6 months ago

berendbaas commented 8 months ago

I recently ran into issues while building Polyscope on my Fedora 39 machine, using both Clang and GCC.

The issue is caused by unknown cstdint types. One of the example errors:

/project/deps/polyscope/src/../include/polyscope/internal.h:20:1: error: ‘uint64_t’ does not name a type
   20 | uint64_t getNextUniqueID();
      | ^~~~~~~~

The issue seems to be that uint64_t is not explicitly included from cstdint, and the default inclusion of it was (probably?) an artifact of different compilers.

The Porting to GCC 13 page implies this issue under "Header dependency changes". I've not been able to trace it down in the changelog for clang, but seems to fail here at least since Clang 15.

Here's a table of tests I ran compiling the polyscope repo in different environments:

OS Compiler Result
Mac OS Ventura 13.6.1 Apple Clang 15.0.0 Compiles
Fedora 39 Clang 17.0.6 Does not compile
Fedora 39 gcc 13 Does not compile
Fedora 39 Clang 15.0.7 Does not compile
Ubuntu 23.10 gcc 12 (12.3.0) Compiles
Ubuntu 23.10 gcc 13 (13.2.0) Does not compile
Ubuntu 23.10 Clang 15.0.7 Does not compile

The preferred solution is probably to explicitly include cstdint where necessary. A (temporary) workaround for me was to force inclusion of the headers in the CMakeLists.txt:

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -include cstdint")
nmwsharp commented 7 months ago

Thank you for this issue, very helpful! (Especially the table of which versions succeed)

I actually couldn't immediately reproduce this with clang-15 on my Ubuntu machine, but that is probably some machine-dependent std library version thing or something. I believe then issue is real, I just couldn't easily test a fix :)

This commit addce9c adds #include <cstdint> everywhere I saw the fixed-width integers being used. Can you confirm if that resolve the issue on your machine?

Longer term, it would be great to add a CI build matrix entry that tests this, although I couldn't immediately figure that out.

berendbaas commented 7 months ago

Glad to be of help & thanks for the quick turnaround.

This works great. I'm able to build on all aforementioned environments now.

With regards to CI, I don't have much experience, but perhaps inspiration can be drawn from libigl's configuration