Add CXX version to precompiled header name

[WardBrian]

This idea was discussed with @rok-cesnovar here: roualdes/bridgestan#151 (comment)

Submisison Checklist

• Run tests: ./runCmdStanTests.py src/test
• Declare copyright holder and open-source license: see below

Summary:

This adds CXX_MAJOR and CXX_MINOR to the name of the precompiled header file when it is enabled. The goal is to prevent the annoying errors from clang updates etc.

Intended Effect:

Prevent clang updates from breaking PCH builds, see cmdstanpy issues

How to Verify:

Side Effects:

This could lead to several PCHs being made over time if the user doesn't run a make clean-all every now and then. This could cause the directory size to increase over time for an installation.

Documentation:

None

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):

Simons Foundation

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

• Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
• Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

[WardBrian]

Note: to really have this work well, we need stan-dev/math#2922

[rok-cesnovar]

Approving but lets way until we have the math PR in. Really hoping this works.

Great idea!!

[WardBrian]

@rok-cesnovar - CI passed for this. Is there any more testing we can do? Maybe try building with a system that has multiple versions of clang installed?

[rok-cesnovar]

I ran this on a system that has both clang-11 and clang-12:

git clone https://github.com/stan-dev/cmdstan --recursive
make clean-all

sudo ln -s /usr/bin/clang-11 /usr/bin/clang
sudo ln -s /usr/bin/clang++-11 /usr/bin/clang++

make build -j8
make examples/bernoulli/bernoulli

sudo rm /usr/bin/clang
sudo rm /usr/bin/clang++
sudo ln -s /usr/bin/clang-12 /usr/bin/clang
sudo ln -s /usr/bin/clang++-12 /usr/bin/clang++

touch examples/bernoulli/bernoulli.stan
make examples/bernoulli/bernoulli

ls stan/src/stan/model/

  finite_diff_grad.hpp           gradient.hpp             hessian_times_vector.hpp  log_prob_grad.hpp    model_base_crtp.hpp   model_header_11_0.d        model_header_12_0.hpp.gch  test_gradients.hpp
grad_hess_log_prob.hpp         gradient_dot_vector.hpp  indexing                  log_prob_propto.hpp  model_functional.hpp  model_header_11_0.hpp.gch  prob_grad.hpp
grad_tr_mat_times_hessian.hpp  hessian.hpp              indexing.hpp              model_base.hpp       model_header.hpp      model_header_12_0.d        rethrow_located.hpp

Cleaning up works great as well. The only question I have is:

• is "_12_0" enough of a detailed version to notably make this precompiled headers experience better?

[WardBrian]

is "_12_0" enough of a detailed version to notably make this precompiled headers experience better?

One question which might help answer that is "Can you run that same experiment against develop?"
My understanding is the answer is no, compiling with clang-12 would fail unless you run a make clean-all beforehand. Assuming I'm correct, at a bare minimum this seems to be an improvement.

There's a related question which is "could/should we be doing more?", which I'm less confident about. If we really wanted to we could try to put the compiler patch version (e.g. 12.0.x) in there as well

[rok-cesnovar]

On develop it failed with:

make examples/bernoulli/bernoulli

--- Translating Stan model to C++ code ---
bin/stanc  --o=examples/bernoulli/bernoulli.hpp examples/bernoulli/bernoulli.stan

--- Compiling, linking C++ code ---
clang++ -std=c++1y -D_REENTRANT -Wno-sign-compare -Wno-ignored-attributes      -I stan/lib/stan_math/lib/tbb_2020.3/include    -O3 -I src -I stan/src -I stan/lib/rapidjson_1.1.0/ -I lib/CLI11-1.9.1/ -I stan/lib/stan_math/ -I stan/lib/stan_math/lib/eigen_3.4.0 -I stan/lib/stan_math/lib/boost_1.78.0 -I stan/lib/stan_math/lib/sundials_6.1.1/include -I stan/lib/stan_math/lib/sundials_6.1.1/src/sundials    -DBOOST_DISABLE_ASSERTS          -c -include-pch stan/src/stan/model/model_header.hpp.gch -x c++ -o examples/bernoulli/bernoulli.o examples/bernoulli/bernoulli.hpp
error: Objective-C was enabled in PCH file but is currently disabled
error: PCH file was compiled for the tune CPU '' but the current translation unit is being compiled for target 'generic

So I agree, this is an improvement and lets merge it as is. We will see if a patch version is needed once this fix is out in the wild.