Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build with gcc13 #3255

Merged
merged 1 commit into from
Jan 18, 2024
Merged

Fix build with gcc13 #3255

merged 1 commit into from
Jan 18, 2024

Conversation

iyanmv
Copy link
Contributor

@iyanmv iyanmv commented Jan 17, 2024

Submission Checklist

  • Run unit tests: ./runTests.py src/test/unit
  • Run cpplint: make cpplint
  • Declare copyright holder and open-source license: see below

Summary

With GCC 13, some C++ Standard Library headers have been changed to no longer include other headers that were being used internally by the library (see, e.g. this). This has affected many other projects.

I noticed this when trying to run the tests with:
python runTests.py -j$(nproc) src/test

The fix is simple, <cstdint> has to be included manually.

Intended Effect

Fix build with g++13

How to Verify

Run the tests

Side Effects

None?

Documentation

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company):

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@WardBrian
Copy link
Member

@serban-nicusor-toptal any idea why this wouldn't have been caught by the bleeding-edge pipeline?

@WardBrian
Copy link
Member

Oh, something else probably includes cstdint when we're doing complete CmdStan builds, which is why we missed it

@serban-nicusor-toptal
Copy link
Contributor

Hey, I'm not sure. Just ran a fresh build, making sure everything is built with the latest, and checked versions afterward, looks fine there.

@iyanmv
Copy link
Contributor Author

iyanmv commented Jan 18, 2024

I'm using g++ (GCC) 13.2.1 20230801 with

#-- Compiler and Linker Flags
#CPPFLAGS=""
CFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions \
        -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security \
        -fstack-clash-protection -fcf-protection"
CXXFLAGS="$CFLAGS -Wp,-D_GLIBCXX_ASSERTIONS"
LDFLAGS="-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now"
LTOFLAGS="-flto=auto"

@iyanmv
Copy link
Contributor Author

iyanmv commented Jan 18, 2024

Oh, wait... I just realized that the python script is downloading from https://github.com/stan-dev/stanc3/releases/download/nightly/linux-stanc instead of using the compiled ones. So I cannot use that in my PKGBUILD.

Let me rerun the tests manually.

@iyanmv
Copy link
Contributor Author

iyanmv commented Jan 18, 2024

In any case, here is the error when I use the python script:

g++ -march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions         -Wp,-D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security         -fstack-clash-protection -fcf-protection -Wp,-D_GLIBCXX_ASSERTIONS -std=c++1y -pthread -D_REENTRANT -Wno-sign-compare -Wno-ignored-attributes      -I lib/stan_math/lib/tbb_2020.3/include    -O3 -I src -I . -I lib/rapidjson_1.1.0/ -I lib/stan_math/ -I lib/stan_math/lib/eigen_3.4.0 -I lib/stan_math/lib/boost_1.81.0 -I lib/stan_math/lib/sundials_6.1.1/include -I lib/stan_math/lib/sundials_6.1.1/src/sundials -I lib/stan_math/lib/benchmark_1.5.1/googletest/googletest/include -I lib/stan_math/lib/benchmark_1.5.1/googletest/googletest -I lib/stan_math/lib/benchmark_1.5.1/googletest/googletest/include -I lib/stan_math/lib/benchmark_1.5.1/googletest/googletest      -DBOOST_DISABLE_ASSERTS         -DGTEST_HAS_PTHREAD=0 -DGTEST_HAS_PTHREAD=0  -c src/test/unit/io/json/json_data_handler_test.cpp -o test/unit/io/json/json_data_handler_test.o
In file included from src/stan/io/json/json_data_handler.hpp:5,
                 from src/stan/io/json/json_data.hpp:4,
                 from src/test/unit/io/json/json_data_handler_test.cpp:1:
src/stan/io/json/json_handler.hpp:97:38: error: ‘uint64_t’ has not been declared
   97 |   virtual void number_unsigned_int64(uint64_t n) {}
      |                                      ^~~~~~~~
make: *** [make/tests:64: test/unit/io/json/json_data_handler_test.o] Error 1
make: *** Waiting for unfinished jobs....

@iyanmv
Copy link
Contributor Author

iyanmv commented Jan 18, 2024

This is probably a different question/issue, but what is the right way of compiling CmdStan and run all tests (including stan and stan_math) with the binaries that were generated when compiling CmdStan (and not precompiled ones from Github). I think the issue is that the Makefile in (stan/make/tests) does not try to use the binaries in the parent folder (cmdstan/bin).

@andrjohns
Copy link
Contributor

It looks like this is also an issue with clang++-17. I just had a CRAN submission fail with this error on Debian & Clang 17:

clang++-17  -std=gnu++17 -I"/home/hornik/tmp/R/include" -DNDEBUG -I"../inst/include" -D_REENTRANT -DSTRICT_R_HEADERS -D_HAS_AUTO_PTR_ETC=0 -DEIGEN_PERMANENTLY_DISABLE_STUPID_WARNINGS -I'/home/hornik/lib/R/Library/4.4/x86_64-linux-gnu/Rcpp/include' -I'/home/hornik/lib/R/Library/4.4/x86_64-linux-gnu/RcppEigen/include' -I'/home/hornik/lib/R/Library/4.4/x86_64-linux-gnu/BH/include' -I'/home/hornik/lib/R/Library/4.4/x86_64-linux-gnu/RcppParallel/include' -I'/home/hornik/lib/R/Library/4.4/x86_64-linux-gnu/rapidjsonr/include' -I/usr/local/include -DUSE_TYPE_CHECKING_STRICT -D_FORTIFY_SOURCE=3   -fpic  -g -O3 -Wall -pedantic -c model_methods.cpp -o model_methods.o
In file included from model_methods.cpp:1:
In file included from ../inst/include/stan/io/json/json_data.hpp:4:
In file included from ../inst/include/stan/io/json/json_data_handler.hpp:5:
../inst/include/stan/io/json/json_handler.hpp:97:38: error: unknown type name 'uint64_t'
   97 |   virtual void number_unsigned_int64(uint64_t n) {}

@WardBrian
Copy link
Member

@serban-nicusor-toptal how hard would it be to add the Stan unit tests to our bleeding-edge-compilers pipeline? So it would be Math unit (gcc/clang) -> Stan unit (gcc/clang) -> building a bunch of models (gcc/clang)

Copy link
Member

@WardBrian WardBrian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@WardBrian WardBrian merged commit d11dea3 into stan-dev:develop Jan 18, 2024
3 checks passed
@serban-nicusor-toptal
Copy link
Contributor

@serban-nicusor-toptal how hard would it be to add the Stan unit tests to our bleeding-edge-compilers pipeline? So it would be Math unit (gcc/clang) -> Stan unit (gcc/clang) -> building a bunch of models (gcc/clang)

Should be fairly simple I hope, will give it a try this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants