Skip to content

Python C++17 Fix, 1.0 branch (2019.08.05.)

Attila Krasznahorkay requested to merge akraszna/atlasexternals:PythonCxx17Fix-1.0-20190805 into 1.0

This update may seem simple at first, but it made me go pretty deep into the rabbit hole today, so I'm tagging a few people on it...

The problems started with my recent CentOS 7 + GCC 8 compatibility updates. While doing that, I had to enable C++17 compilation with "new" compilers all across the board. As part of this, I actually realised that I never told the Boost build to use any particular C++ standard. Since in some local tests (with Gaudi) I actually ran into some issues where Boost had to use the same standard as the Gaudi build, Boost is now being built "correctly" in our latest analysis releases. (bb778fd3)

And it was when I tried to build AnalysisBase-21.2.83 on macOS that I ran into a new kind of build error.

clang-darwin.compile.c++ bin.v2/libs/python/build/clang-darwin-10.0/release/python-2.7/threading-multi/visibility-hidden/list.o
In file included from libs/python/src/list.cpp:5:
In file included from ./boost/python/list.hpp:8:
In file included from ./boost/python/detail/prefix.hpp:13:
In file included from ./boost/python/detail/wrap_python.hpp:151:
In file included from /Users/krasznaa/ATLAS/sw/projects/volatile/AnalysisBase/build/x86_64-mac1014-clang100-opt/include/python2.7/Python.h:88:
/Users/krasznaa/ATLAS/sw/projects/volatile/AnalysisBase/build/x86_64-mac1014-clang100-opt/include/python2.7/unicodeobject.h:534:5: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
    register PyObject *obj,     /* Object */
    ^~~~~~~~~
/Users/krasznaa/ATLAS/sw/projects/volatile/AnalysisBase/build/x86_64-mac1014-clang100-opt/include/python2.7/unicodeobject.h:553:5: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
    register PyObject *obj      /* Object */
    ^~~~~~~~~
/Users/krasznaa/ATLAS/sw/projects/volatile/AnalysisBase/build/x86_64-mac1014-clang100-opt/include/python2.7/unicodeobject.h:575:5: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
    register const wchar_t *w,  /* wchar_t buffer */
    ^~~~~~~~~
/Users/krasznaa/ATLAS/sw/projects/volatile/AnalysisBase/build/x86_64-mac1014-clang100-opt/include/python2.7/unicodeobject.h:593:5: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
    register wchar_t *w,        /* wchar_t buffer */

Which, I found with a short google-ing, is a pretty well known error.

For a minute I actually considered trying to backport the full Python 3 update into Python 2.7, but that only lasted for about 10 seconds. 😛 Since the Python-2.7.15 code is riddled with that keyword. So in this MR I only limited myself to removing it from the public headers. Which is actually a robust solution to this particular issue.

  • All of this code is in pure C code, which Python compiles with the configured C compiler, not with the C++ one;
  • Since the Python build happens from C source files, a C++ standard is not set up anywhere during the Python build. (And neither should it be.)

So as long as the headers of Python are okay, we can use Clang. And fix the macOS build of AnalysisBase/AnalysisTop.

And now comes the rabbit hole part. I was curious what LCG was doing with this. Since for LCG_96 we do have a Clang based build.

http://lcginfo.cern.ch/release_packages/x86_64-centos7-clang8-opt/96/

As you would expect, I found that Boost is not being built using the correct C++ standard as part of LCG. 😦 And apparently nobody is using that platform yet, otherwise they would've run into this issue already, just as CMS did when they tried to build their code with Clang...

So let me tag a few people. @krumnack, @wlampl, @emoyse, @mato, @ganis. I believe we may want to introduce the same sort of patch into the future LCG builds as well. Also, we should start building the Boost shared libraries with the correct C++ standard...

Merge request reports