MySQL Bugs: #89420: Enforcing C++03 mode in non debug builds

Bug #89420	Enforcing C++03 mode in non debug builds
Submitted:	25 Jan 2018 16:31	Modified:	31 Jan 2020 13:42
Reporter:	Zsolt Parragi (OCA)	Email Updates:
Status:	Won't fix	Impact on me:	None
Category:	MySQL Server: Compiling	Severity:	S3 (Non-critical)
Version:	5.5, 5.6, 5.7	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	Contribution

Description:
5.7 contains the following in cmake/build_configurations/compiler_options.cmake:

    IF(GXX_VERSION VERSION_EQUAL 6.0 OR GXX_VERSION VERSION_GREATER 6.0)
      SET(COMMON_CXX_FLAGS             "${COMMON_CXX_FLAGS} -std=gnu++03")
    ENDIF()

For 5.5 and 5.7, I can't find anything similar.

This basically means, that when using new gcc versions for debug and relwithdebinfo builds, MySQL is built in C++03 mode with gcc extensions. When using release builds, or when using different compilers (e.g. clang), it's built in the compiler's default mode (C++14 by default for newer compilers)

I think using different C++ standards for debug and release builds is problematic, as it could result in semantic differences. Especially since even the reldebinfo build is built with a different standard than the release build, which means that basically any debugging happens compiled with a different standard.

Instead of enforcing the standard only on linux using a manually specified flag, the standard CMake variable, CMAKE_CXX_STANDARD could be used for any compilers:

SET(CMAKE_CXX_STANDARD 98)

If MySQL really needs the gnu03 variant for some reason, it could be specified for linux builds:

SET(CMAKE_CXX98_EXTENSION_COMPILE_OPTION -std=gnu++03)

While this variable is only supported in CMake 3.1+, I think it is unlikely that somebody would use an older cmake with gcc 6 or newer.

How to repeat:
Compile mysql with a new gcc/clang version

Use the same c++ standard mode on all linux builds (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: cpp-standard.diff (text/x-patch), 4.75 KiB.

Hello Zsolt Parragi ,

Thank you for the report and contribution.

Thanks,
Umesh

Posted by developer:
 
Hi Zsolt,

Thank youfor your contribution, but we are unfortunately not able to take it.

We have to use native compilers on all unix platforms.
The oldest gcc version for MySQL 5.6 is gcc 4.4 for RedHat Linux.

We build 5.6 on Solaris with /opt/studio12u2/bin/CC and libstlport.
We build 5.6 on windows with VS 2010

I'm afraid that if we enable C++03 in 5.6 for gcc/clang, we risk introducing
incompatibilities with Sun Studio and/or Visual Studio, and break the
build on those platforms.

Compiler versions are also affected by the language standards
we have in different branches, and vice versa:

- 5.5 is C+-      i.e. not really C++ at all (any use of std:: is forbidden)
- 5.6 is C++98
- 5.7 is C++03
- 8.0 is C++11 which requires at least Visual Studio 2015, gcc 4.8

So newer servers will not build with older compilers.

The other way around is maintained though. I build all branches with
Visual Studio 2015. We try to ensure that older branches will build
with new compilers

We do try to ensure that older branches build with newer compilers (this
typically means backporting fixes for various compiler warnings)

It is unclear to me whether your contributed patch is intended for 5.5
or 5.6 or 5.7 (there's a typo I think: "For 5.5 and 5.7, I can't find
anything similar.")

It seems that CMAKE_CXX_STANDARD was introduced in cmake 3.1.3 so it
won't have any effect unless you have a modern cmake. MySQL 5.7 has
CMAKE_MINIMUM_REQUIRED(VERSION 2.8.9) Maybe we can assume that you
have a modern cmake on your system if you have GCC6? I don't know.

I cannot find documentation on CMAKE_CXX98_EXTENSION_COMPILE_OPTION
I do find it in the source code for cmake 3.1 though.

Finally: we claim in our cmake code:
SET(BUILDTYPE_DOCSTRING
 "Choose the type of build, options are: None(CMAKE_CXX_FLAGS or
 CMAKE_C_FLAGS used) Debug Release RelWithDebInfo MinSizeRel")

But this isn't really true. We use only Debug and RelWithDebInfo
for our testing. Anything else is untested, and hence unsupported. Adding
support for other build types wouldn't be very hard to do, but will in
practice be impossible to maintain: we build on literally dozens of
different platforms, and we simply don't have the capacity to
test/maintain even more build types.

BR,
Ståle

Hello

Yes, sorry, there was a typo in my description: the code is from 5.7, I couldn't find anything in 5.5 or 5.6. I din't know that there was a difference, that's why I mentioned also 3 versions - and also because gcc6 does default to C++14.

Which is why it looks strange to me, that:
(1) With gcc6, you build 5.5 and 5.6 in C++14 mode
(2) You also build your release build in C++14 mode for 5.7
(3) But for Debug and RelWithDebInfo, you build it with C++03 with gnu extensions
(4) And you say that 5.7 defaults to C++03, but gcc before 5 defaults to C++98 - so with older compilers, you are actually using C++98, not C++03, even for 5.7.

I guess that the setting for (3), which I changed here, is there because of the MAINTAINER_MODE: without that, auto_ptr results in a long list of deprecation warnings.
Still, I find it strange that you are running the tests in debug and relwithdebinfo, which uses a different c++ standard compared to your real release - that's why I'm suggesting to fix it in a way which also changes the release build to use the same standard.

My other reason for this is that your current check is GCC specific, which means maintainer mode doesn't work with clang.

About modern cmake: you are right, I made the assumption that nobody will use a recent compiler with an ancient cmake version. That could be incorrect, in which case, I could change the diff to do not depend on that property, unless the cmake version is high enough.

Posted by developer:
 
>> Yes, sorry, there was a typo in my description: the code is from 5.7, I
>> couldn't find anything in 5.5 or 5.6. I din't know that there was a
>> difference, that's why I mentioned also 3 versions - and also because gcc6
>> does default to C++14.
>> 
>> Which is why it looks strange to me, that:
>> (1) With gcc6, you build 5.5 and 5.6 in C++14 mode

5.5 and 5.6 simply cannot be built with modern compilers (GCC7 and up)
The most modern compiler we use internally for 5.5/5.6 is GCC 4.8

One thing is you get zillions of warnings. More importantly, the
codebase has quite a few examples of undefined behaviour. We have
found that modern compilers will freely optimize away such code in
certain cases, and hence generate executables that behave badly and/or
simply crash. This is not speculation on my part: when adapting to
GCC8 we did get some surprises in this area, tests started to fail for
no apparent reason. The reason was indeed that code got optimized away.

Maybe I should add an explicit warning about compiler limitations, and
abort the build?

You *should* be able to build 5.7 with modern compilers. We already
use GCC7 on some platforms, and I'm currently porting to GCC8.

>> (2) You also build your release build in C++14 mode for 5.7

We *never* build release builds, only RelWithDebInfo.

>> (3) But for Debug and RelWithDebInfo, you build it with C++03 with gnu
>> extensions

Yes, 5.7 is C++03

>> (4) And you say that 5.7 defaults to C++03, but gcc before 5 defaults to
>> C++98 - so with older compilers, you are actually using C++98, not C++03,
>> even for 5.7.

C++98 / C++03 is mostly the same thing.

>> 
>> I guess that the setting for (3), which I changed here, is there because of
>> the MAINTAINER_MODE: without that, auto_ptr results in a long list of
>> deprecation warnings.
>> Still, I find it strange that you are running the tests in debug and
>> relwithdebinfo, which uses a different c++ standard compared to your real
>> release - that's why I'm suggesting to fix it in a way which also changes the
>> release build to use the same standard.

We assume that you always include compiler_options.cmake when you
build, otherwise you are totally on your own. And to repeat myself:
we *never* use CMAKE_BUILD_TYPE=Release

>> 
>> My other reason for this is that your current check is GCC specific, which
>> means maintainer mode doesn't work with clang.

We have been focusing on gcc only for MAINTAINER_MODE for the last
several years. For the upcoming 8.0 I have had my own pet project to
keep it clean also for Clang (on Mac and Linux)

>> 
>> About modern cmake: you are right, I made the assumption that nobody will use
>> a recent compiler with an ancient cmake version. That could be incorrect, in
>> which case, I could change the diff to do not depend on that property, unless
>> the cmake version is high enough.

For Redhat we have native cmake 2.8.12.2. In MySQL 8.0 we have
switched to use /opt/rh/devtoolset-xx, so indeed: old cmake / new GCC.

The best solution would be to read docs for cmake 2.8.9 only, that's
CMAKE_MINIMUM_REQUIRED in 5.7. Adding a check for clang version,
similar to the one we have for GCC certainly makes sense.