root icon indicating copy to clipboard operation
root copied to clipboard

Published 20 hours ago • verified publisher icon root-project

Switching CMAKE_CXX_STANDARD breaks build

Open hahnjo opened this issue 4 years ago • 10 comments

Describe the bug

The build errors when switching from CMAKE_CXX_STANDARD=14 (the default with GCC 8.4.1 on CentOS 8) to CMAKE_CXX_STANDARD=17 (for example to get ROOT7).

FAILED: tree/treeplayer/G__TreePlayer.cxx lib/TreePlayer.pcm
[...]
In module 'std' imported from input_line_1:1:
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/hashtable.h:2084:4: error: no matching member function for call to '_M_rehash_aux'
          _M_rehash_aux(__n, __unique_keys());
          ^~~~~~~~~~~~~
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/hashtable.h:1730:8: note: in instantiation of member function 'std::_Hashtable<std::__cxx11::basic_string<char>, std::pair<const std::__cxx11::basic_string<char>, std::unique_ptr<ROOT::Internal::TNamedBranchProxy, std::default_delete<ROOT::Internal::TNamedBranchProxy> > >, std::allocator<std::pair<const std::__cxx11::basic_string<char>, std::unique_ptr<ROOT::Internal::TNamedBranchProxy, std::default_delete<ROOT::Internal::TNamedBranchProxy> > > >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char> >, std::hash<std::__cxx11::string>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_rehash' requested here
              _M_rehash(__do_rehash.second, __saved_state);
              ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/hashtable_policy.h:739:16: note: in instantiation of member function 'std::_Hashtable<std::__cxx11::basic_string<char>, std::pair<const std::__cxx11::basic_string<char>, std::unique_ptr<ROOT::Internal::TNamedBranchProxy, std::default_delete<ROOT::Internal::TNamedBranchProxy> > >, std::allocator<std::pair<const std::__cxx11::basic_string<char>, std::unique_ptr<ROOT::Internal::TNamedBranchProxy, std::default_delete<ROOT::Internal::TNamedBranchProxy> > > >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char> >, std::hash<std::__cxx11::string>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_insert_unique_node' requested here
          return __h->_M_insert_unique_node(__n, __code, __p)->second;
                      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/unordered_map.h:978:16: note: in instantiation of member function 'std::__detail::_Map_base<std::__cxx11::basic_string<char>, std::pair<const std::__cxx11::basic_string<char>, std::unique_ptr<ROOT::Internal::TNamedBranchProxy, std::default_delete<ROOT::Internal::TNamedBranchProxy> > >, std::allocator<std::pair<const std::__cxx11::basic_string<char>, std::unique_ptr<ROOT::Internal::TNamedBranchProxy, std::default_delete<ROOT::Internal::TNamedBranchProxy> > > >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char> >, std::hash<std::__cxx11::string>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true>, true>::operator[]' requested here
      { return _M_h[std::move(__k)]; }
               ^
/home/jhahnfel/ROOT/build/include/TTreeReader.h:264:15: note: in instantiation of member function 'std::unordered_map<std::__cxx11::basic_string<char>, std::unique_ptr<ROOT::Internal::TNamedBranchProxy, std::default_delete<ROOT::Internal::TNamedBranchProxy> >, std::hash<std::__cxx11::string>, std::equal_to<std::__cxx11::basic_string<char> >, std::allocator<std::pair<const std::__cxx11::basic_string<char>, std::unique_ptr<ROOT::Internal::TNamedBranchProxy, std::default_delete<ROOT::Internal::TNamedBranchProxy> > > > >::operator[]' requested here
      fProxies[bpName].reset(p);
              ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/hashtable.h:915:12: note: candidate function not viable: no known conversion from 'integral_constant<...>' to 'integral_constant<...>' for 2nd argument
      void _M_rehash_aux(size_type __n, std::true_type);
           ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/hashtable.h:918:12: note: candidate function not viable: no known conversion from 'integral_constant<[...], true aka true>' to 'integral_constant<[...], false>' for 2nd argument
      void _M_rehash_aux(size_type __n, std::false_type);
           ^
Error: /home/jhahnfel/ROOT/build/bin/rootcling: compilation failure (/home/jhahnfel/ROOT/build/lib/libTreePlayer8033212c3f_dictUmbrella.h)

and

FAILED: montecarlo/pythia8/G__EGPythia8.cxx lib/EGPythia8.pcm
[...]
In module 'std' imported from input_line_1:1:
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:601:2: error: no matching member function for call to '_M_move_assign'
        _M_move_assign(std::move(__x), __bool_constant<__move_storage>());
        ^~~~~~~~~~~~~~
/usr/include/Pythia8/LHEF3.h:212:25: note: in instantiation of member function 'std::vector<Pythia8::XMLTag *, std::allocator<Pythia8::XMLTag *> >::operator=' requested here
      tags.back()->tags = findXMLTags(tags.back()->contents, &leftovers);
                        ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1677:7: note: candidate function not viable: no known conversion from 'integral_constant<...>' to 'integral_constant<...>' for 2nd argument
      _M_move_assign(vector&& __x, std::true_type) noexcept
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1688:7: note: candidate function not viable: no known conversion from 'integral_constant<[...], true aka true>' to 'integral_constant<[...], false>' for 2nd argument
      _M_move_assign(vector&& __x, std::false_type)
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:601:2: error: no matching member function for call to '_M_move_assign'
        _M_move_assign(std::move(__x), __bool_constant<__move_storage>());
        ^~~~~~~~~~~~~~
/usr/include/Pythia8/Settings.h:129:7: note: in instantiation of member function 'std::vector<int, std::allocator<int> >::operator=' requested here
class MVec {
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1677:7: note: candidate function not viable: no known conversion from 'integral_constant<...>' to 'integral_constant<...>' for 2nd argument
      _M_move_assign(vector&& __x, std::true_type) noexcept
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1688:7: note: candidate function not viable: no known conversion from 'integral_constant<[...], true aka true>' to 'integral_constant<[...], false>' for 2nd argument
      _M_move_assign(vector&& __x, std::false_type)
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:601:2: error: no matching member function for call to '_M_move_assign'
        _M_move_assign(std::move(__x), __bool_constant<__move_storage>());
        ^~~~~~~~~~~~~~
/usr/include/Pythia8/Settings.h:152:7: note: in instantiation of member function 'std::vector<double, std::allocator<double> >::operator=' requested here
class PVec {
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1677:7: note: candidate function not viable: no known conversion from 'integral_constant<...>' to 'integral_constant<...>' for 2nd argument
      _M_move_assign(vector&& __x, std::true_type) noexcept
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1688:7: note: candidate function not viable: no known conversion from 'integral_constant<[...], true aka true>' to 'integral_constant<[...], false>' for 2nd ar
gument
      _M_move_assign(vector&& __x, std::false_type)
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:601:2: error: no matching member function for call to '_M_move_assign'
        _M_move_assign(std::move(__x), __bool_constant<__move_storage>());
        ^~~~~~~~~~~~~~
/usr/include/Pythia8/Settings.h:175:7: note: in instantiation of member function 'std::vector<std::__cxx11::basic_string<char>, std::allocator<std::__cxx11::basic_string<char> > >::operator=' requested here
class WVec {
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1677:7: note: candidate function not viable: no known conversion from 'integral_constant<...>' to 'integral_constant<...>' for 2nd argument
      _M_move_assign(vector&& __x, std::true_type) noexcept
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1688:7: note: candidate function not viable: no known conversion from 'integral_constant<[...], true aka true>' to 'integral_constant<[...], false>' for 2nd argument
      _M_move_assign(vector&& __x, std::false_type)
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:601:2: error: no matching member function for call to '_M_move_assign'
        _M_move_assign(std::move(__x), __bool_constant<__move_storage>());
        ^~~~~~~~~~~~~~
/usr/include/Pythia8/HelicityBasics.h:240:9: note: in instantiation of member function 'std::vector<std::vector<std::complex<double>, std::allocator<std::complex<double> > >, std::allocator<std::vector<std::complex<double>, std::allocator<std::complex<double> > > > >::operator=' requested here
    rho = vector< vector<complex> >(spinStates(),
        ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1677:7: note: candidate function not viable: no known conversion from 'integral_constant<...>' to 'integral_constant<...>' for 2nd argument
      _M_move_assign(vector&& __x, std::true_type) noexcept
      ^
/usr/lib/gcc/x86_64-redhat-linux/8/../../../../include/c++/8/bits/stl_vector.h:1688:7: note: candidate function not viable: no known conversion from 'integral_constant<[...], true aka true>' to 'integral_constant<[...], false>' for 2nd argument
      _M_move_assign(vector&& __x, std::false_type)
      ^
Error: /home/jhahnfel/ROOT/build/bin/rootcling: compilation failure (/home/jhahnfel/ROOT/build/lib/libEGPythia898309100d9_dictUmbrella.h)

Expected behavior

The build should succeed.

To Reproduce

Configure current master with cmake -DCMAKE_CXX_STANDARD=14 (or on a platform where this is detected as the default) and then switch to CMAKE_CXX_STANDARD=17.

hahnjo avatar Jul 12 '21 08:07 hahnjo

Ok, sorry for much of the noise here. The issue only happens when switching CMAKE_CXX_STANDARD which doesn't rebuild std.pcm, directly setting the right value works fine. Assigning to Oksana because it would be nice if the build system could rebuild std.pcm if the value of -std=c++XY changed, but feel free to close if not feasible.

hahnjo avatar Jul 12 '21 10:07 hahnjo

Build system ownership went to Bertrand; reassigned.

Axel-Naumann avatar Jul 12 '21 12:07 Axel-Naumann

Yes, let's maybe close this issue. We don't support C++ 14 anymore, and this problem occurred when changing from C++14 to C++17.

If there are new issues with the currently-supported C++ standards that you want to get fixed, feel free to open a new issue!

guitargeek avatar Apr 05 '24 01:04 guitargeek

FWIW the same problem can be seen when going from CMAKE_CXX_STANDARD=17 to CMAKE_CXX_STANDARD=20 with GCC 13.2.0:

In module 'std' imported from input_line_1:1:
/opt/gcc/13.2.0/lib/gcc/x86_64-pc-linux-gnu/13.2.0/../../../../include/c++/13.2.0/type_traits:1101:21: error: static assertion expression is not an integral constant expression
      static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/gcc/13.2.0/lib/gcc/x86_64-pc-linux-gnu/13.2.0/../../../../include/c++/13.2.0/bits/unique_ptr.h:237:13: note: in instantiation of template class 'std::is_move_constructible<std::default_delete<ROO
T::Internal::RHashMap>>' requested here
            bool = is_move_constructible<_Dp>::value,
                   ^
/opt/gcc/13.2.0/lib/gcc/x86_64-pc-linux-gnu/13.2.0/../../../../include/c++/13.2.0/bits/unique_ptr.h:283:7: note: in instantiation of default argument for '__uniq_ptr_data<ROOT::Internal::RHashMap, std
::default_delete<ROOT::Internal::RHashMap>>' required here
      __uniq_ptr_data<_Tp, _Dp> _M_t;
      ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/jhahnfel/ROOT/build-asserts-gcc13/include/ROOT/RConcurrentHashColl.hxx:32:38: note: in instantiation of template class 'std::unique_ptr<ROOT::Internal::RHashMap>' requested here
   mutable std::unique_ptr<RHashMap> fHashMap;
                                     ^
/opt/gcc/13.2.0/lib/gcc/x86_64-pc-linux-gnu/13.2.0/../../../../include/c++/13.2.0/type_traits:1101:21: note: non-literal type 'true_type' (aka 'integral_constant<bool, true>') cannot be used in a cons
tant expression
      static_assert(std::__is_complete_or_unbounded(__type_identity<_Tp>{}),
                    ^

I understand the desire to reduce the number of issues, but closing fully valid reports IMHO sends the wrong signal...

hahnjo avatar Apr 05 '24 09:04 hahnjo

@hahnjo, you were opening this issue as "Build broken with C++17 on CentOS 8", and then only renamed it later to just because you understood the build was not actually broken "Switching CMAKE_CXX_STANDARD breaks build". This kind of re-interpretation biases the relevance of the issue, which is why I'm not a big fan of it and was trigger happy to close it.

When reinterpreting issues like this, you have to ask yourself: would I have also opened an original issue for this too? Especially because of:

The build errors when switching from CMAKE_CXX_STANDARD=14 (the default with GCC 8.4.1 on CentOS 8) to CMAKE_CXX_STANDARD=17 (for example to get ROOT7).

ROOT 7 has now the same CMAKE_CXX_STANDARD requirements, so please give me a new example of why someone would do this.

Anyone who reads this issue to consider fixing it will have this context in mind, which brings them to the conclusion that it's not relevant to fix. So why keep it open?

guitargeek avatar Apr 10 '24 08:04 guitargeek

I understand the desire to reduce the number of issues, but closing fully valid reports IMHO sends the wrong signal...

I'm very happy to discuss about this by the way and I would like to understand you better and improve my communication when closing issues: what "wrong signal" did it send in this case?

guitargeek avatar Apr 10 '24 08:04 guitargeek

This kind of re-interpretation biases the relevance of the issue, which is why I'm not a big fan of it and was trigger happy to close it.

Acknowledged that re-interpretation of issues is difficult; in this case I would argue that I better understood the underlying reason and changed the issue to correct the description. Just closing and dismissing this analysis is what I'm against.

When reinterpreting issues like this, you have to ask yourself: would I have also opened an original issue for this too? Especially because of:

Yes, I would have opened the same issue if I had understood the underlying reason before; that's why I updated it.

ROOT 7 has now the same CMAKE_CXX_STANDARD requirements, so please give me a new example of why someone would do this.

I already gave you an example in https://github.com/root-project/root/issues/8642#issuecomment-2039323384:

FWIW the same problem can be seen when going from CMAKE_CXX_STANDARD=17 to CMAKE_CXX_STANDARD=20 with GCC 13.2.0:

C++17 is now the default, but if somebody tries to switch an existing build directory to C++20, they will be faced with obscure error messages.

Anyone who reads this issue to consider fixing it will have this context in mind, which brings them to the conclusion that it's not relevant to fix. So why keep it open?

Anyone looking at this issue should have a look at the entire discussion, including https://github.com/root-project/root/issues/8642#issuecomment-2039323384 and this comment. This should explain why I think it's still relevant.

I'm very happy to discuss about this by the way and I would like to understand you better and improve my communication when closing issues: what "wrong signal" did it send in this case?

The "wrong signal" that this sends is exactly what you describe: A valid report of something not working is thought "not relevant to fix". IMHO this is a disastrous attitude and will just discourage people from reporting issue - it just risks being deemed "not relevant" by the core developers, so why put in the effort?

hahnjo avatar Apr 10 '24 08:04 hahnjo

I already gave you an example in https://github.com/root-project/root/issues/8642#issuecomment-2039323384:

I meant not an example of a crash, but an example of a user story. But you provided that now with the switch from the default C++17 to C++20 because they want new features.

Yes, I would have opened the same issue if I had understood the underlying reason before; that's why I updated it.

Ok, then I apologize for closing it and let's leave it open.

The "wrong signal" that this sends is exactly what you describe: A valid report of something not working is thought "not relevant to fix". IMHO this is a disastrous attitude and will just discourage people from reporting issue - it just risks being deemed "not relevant" by the core developers, so why put in the effort?

Point taken!

guitargeek avatar Apr 10 '24 08:04 guitargeek

The issue looks related to old versions of the standard and compiler. Can we close the issue (won't fix or not relevant)?

dpiparo avatar May 17 '24 13:05 dpiparo

Please see the discussion immediately above; in particular this is still a problem with CMAKE_CXX_STANDARD=20: https://github.com/root-project/root/issues/8642#issuecomment-2039323384

hahnjo avatar May 17 '24 13:05 hahnjo