ryzom / ryzomcore

Ryzom Core is the open-source project related to the Ryzom game. This community repository is synchronized with the Ryzom Forge repository, based on the Core branch.
https://wiki.ryzom.dev
GNU Affero General Public License v3.0
333 stars 90 forks source link

Wrong C++ standard flag #329

Closed ryzom-pipeline closed 6 years ago

ryzom-pipeline commented 6 years ago

Original report by Rodolphe Breard (Bitbucket: rodolphe_breard, ).


Hi,

As you know, there is several C++ standards and there is a compilation flag, -std which defines which one the compiler should use. For RyzomCore, this flag is set in code/CMakeModules/nel.cmake at line 877:

# use c++0x standard to use std::unique_ptr and std::shared_ptr
SET(PLATFORM_CXXFLAGS "${PLATFORM_CXXFLAGS} -std=c++0x")

After a compilation error which I didn't had before with the exact same code, I started to accuse this flag. For a start, c++0x is the deprecated name for c++11.

Here is some tests with different standards using my brand new GCC 8.1.0.

c++98 and gnu++98

So much errors I couldn't find anything suitable to show, my terminal if full of red. RC has clearly evolved since this old standard, which is nice.

c++11 (aka c++0x, the default one)

In file included from /usr/include/c++/8.1.0/map:62,
                 from /home/rodolphe/ryzom/build/nel/src/3d/nel3d_pch/std3d.h:27:
/usr/include/c++/8.1.0/bits/stl_multimap.h: In instantiation of ‘class std::multimap<unsigned int, NL3D::CPSLocatedBindable*, std::less<unsigned int>, std::allocator<unsigned int> >’:
/home/rodolphe/ryzom/ryzomcore/code/nel/include/nel/3d/particle_system.h:1179:19:   required from here
/usr/include/c++/8.1.0/bits/stl_multimap.h:121:21: error: static assertion failed: std::multimap must have the same value_type as its allocator
       static_assert(is_same<typename _Alloc::value_type, value_type>::value,
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Fail.

gnu++11

At last: it works! Yay!

c++14

Same failure as c++11.

gnu++14

Success!

c++17 and gnu++17

C++11 deprecated dynamic exception specifications, which led to a ton of warnings. It is now forbidden and therefore cannot compile.

Conclusion

It seems that RyzomCore uses some GNU extensions. I have no problem with that, however not indicating that in the standard leads to a compilation error. Currently, only gnu++11 and gnu++14 works. gnu++17 should works providing the current deprecation warnings are fixed. I think changing c++0x to gnu++14 is the way to go. Then, when someone has some free time and fixes those issues with exceptions, it should be easy to upgrade to gnu++17.

ryzom-pipeline commented 6 years ago

Original comment by KarolTrzeszczkowski (Bitbucket: KarolTrzeszczkowski, GitHub: KarolTrzeszczkowski).


ryzom-client is not compiling from AUR repo on Arch linux. It returns this exact error -

#!c++

 error: static assertion failed: std::multimap must have the same value_type as its allocator
       static_assert(is_same<typename _Alloc::value_type, value_type>::value,
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ryzom-pipeline commented 6 years ago

Original comment by Meelis Mägi (Bitbucket: [Meelis Mägi](https://bitbucket.org/Meelis Mägi), ).


I'm not sure about gnu extensions because most of the code is from VC6 days.

The multimap definition seems to have invalid allocator defined in nel/src/3d/ps_allocator.h

typedef std::multimap<T, U, Pr, std::allocator<T> > M;

it should be

typedef std::multimap<T, U, Pr, std::allocator<std::pair<const T, U> > > M;

which is default for allocator and makes it same as used under macOS

typedef std::multimap<T, U, Pr > M;
ryzom-pipeline commented 6 years ago

Original comment by Cédric Ochs (Bitbucket: [Cédric OCHS](https://bitbucket.org/Cédric OCHS), ).


Please did the commit "9122 (aaf2d868fb3c) Changed: Use CPP11 everywhere if supported" fix your problem ?

I succeeded to compile Ryzom with GCC 8.0.

ryzom-pipeline commented 6 years ago

Original comment by Jan Boon (Bitbucket: [Jan Boon](https://bitbucket.org/Jan Boon), ).


There's a new syntax in CMake. This will set the C++ standard to C++11 wherever supported, and to the highest available if C++11 is not present on the selected toolchain.

CMAKE_MINIMUM_REQUIRED(VERSION 3.1 FATAL_ERROR) SET(CMAKE_CXX_STANDARD 11)