CMakeLists.txt: Require C++11 and cmake 3.7 #653

t-b · 2020-01-11T02:02:04Z

Fix CI fallout
Remove C++ feature #ifdef handling in tango_config.h
Use cmake testing for cxx_relaxed_constexpr, see also https://cmake.org/cmake/help/latest/prop_gbl/CMAKE_CXX_KNOWN_FEATURES.html#prop_gbl:CMAKE_CXX_KNOWN_FEATURES
Use cmake 3.7 on systems with too old cmake to ensure that it works with this version.
Output cmake version on travis/appveyor
Finalize

Closes #626.

mliszcz · 2020-01-11T11:19:32Z

+1 for this PR, but I'm not sure what is the purpose of checking for relaxed constexpr support if we still need to be compatible with C++11. We are just doubling implementation effort this way (If we need certain computation to happen at compile time we still need to provide #else clause with template-based solution for C++11-only compilers).

t-b · 2020-01-14T16:02:20Z

@mliszcz My idea is that we only use relaxed constexpr and not the C++11 one. The plan is to move to C++14 in august, so I don't think we need to have constexpr for C++11 and C++14.

bourtemb

+1 for this PR too!
Thanks @t-b for the initiative.
Please check my minor review comments.

cppapi/client/api_util.cpp

cppapi/client/devapi_attr.tpp

cppapi/server/attrsetval.tpp

cppapi/server/w_attrsetval.tpp

t-b · 2020-01-20T19:31:12Z

Indentation is fixed.

t-b · 2020-01-22T10:39:53Z

@mliszcz @bourtemb Ready for review. I've not done any automated modernization using clang-tidy/clang-modernize at this point as the PR is already big enough.

And I also did not want to step on everyones toes who has a PR open.

.travis/debian8/Dockerfile

configure/CMakeLists.txt

bourtemb · 2020-01-22T12:43:45Z

What -std flag will be used for compilers that do not support c++14, e.g. gcc 4.8.1)? If I recall, -std=c++11 was never a default setting in gcc and if this is true, the compiler will run in c++98 mode, right? On Debian 8 we compile in c++14 mode so actually there is no job in CI that checks the c++11 mode.

I think
set(CXX_STANDARD_REQUIRED 11)
instruction in CMakeLists.txt is ensuring the code is compiled with at least c++11 enabled.
https://cmake.org/cmake/help/v3.1/prop_tgt/CXX_STANDARD.html#prop_tgt:CXX_STANDARD

t-b · 2020-01-22T12:59:16Z

I'm not 100% sure that set(CXX_STANDARD_REQUIRED 11) will really fail cmake if it the compiler does not understand c++11. But something like the check in #659 does enforce that. And that PR is planned to be merged after this one.

bourtemb

Thanks @t-b for this big PR cleaning up the current code after the decision taken to require C++11 support.
This PR looks good to me as it is now.
It has an impact on a big number of files so this will cause some issues with the other PRs currently being merged which will have to be rebased and conflicts may appear.
I think we should not wait too much before merging this one because it will become quickly a nightmare to keep it up to date otherwise.

cppapi/server/tango_config.h

.travis/debian8/Dockerfile

t-b · 2020-01-22T14:49:42Z

@mliszcz I would merge this one after #640 as your PR is older.

mliszcz · 2020-01-22T14:58:13Z

@t-b Thanks! I've merged the mentioned PR. There is a minor conflict in travis configuration though.

mliszcz

Looks good! Thanks!

We don't use per target properties like cxx_std_11 as these are only supported starting with cmake 3.8. So we set it globally instead.

This is way too old to work anymore as we now require C++11.

We now require C++11. Done with unifdef -DHAS_UNIQUE_PTR

We now require C++11. Done with unifdef -DHAS_MAP_AT

We now require C++11. Done with unifdef -DHAS_OVERRIDE

We now require C++11. Done with unifdef -DHAS_RVALUE

We now require C++11. Done with unifdef -DHAS_THREAD

We now require C++11. Done with unifdef -DHAS_TYPE_TRAITS

We now require C++11. Done with unifdef -DHAS_LAMBDA_FUNC

We now always require C++11.

We now require C++11 so nullptr is always available. But we keep the definitions of Tango_nullptr for backwards compatibility.

We now require C++11. Done with unifdef -DHAS_RANGE_BASE_FOR

We now require C++11. Done with unifdef -DINIT_LIST

We now require C++11. Done with unifdef -DHAS_UNDERLYING

We now always require C++11.

…feature checking We now always require C++11.

This is just too old as it has only GCC 4.7.2.

Exposed with the new C++11 compilation, but ultimately forgotten in cc8f943 (cppapi/server: Ignore warnings for generated code, 2019-11-15).

Found with -Winconsistent-missing-override using latest clang 9.0.

We don't support GCC < 4 anymore.

The only C++ feature we are currently using which is not supported by C++11 is C++14 relaxed constexpr. Instead of using the old school ifdef magic we now use a real feature test. This also requires to move the definition of TANGO_CONSTEXPR to tango_const.h.in.

We now output cmake/compiler and platform versions for easier debugging. Output like the CMAKE_BUILD_TYPE was also changed to be outputted always regardless if default or not.

MSVC versions prior to VS 2015 (aka msvc14) don't have usable C++11 support. So we can't build on those anymore. This was also decided in [2] which said: > Decision was taken to stop support for MSVC9, MSVC10 and MSVC12. The > builds for these MSVC compiler versions will be removed from appveyor. > If some users still need to compile cppTango using these compilers, they > will need to fork cppTango and maintain a version compatible with these > old compilers. [1]: https://docs.microsoft.com/en-us/previous-versions/hh567368(v=vs.140)?redirectedfrom=MSDN#featurelist [2]: https://github.com/tango-controls/tango-kernel-followup/blob/master/2019-02-04/outcomes.md

Removed with unifdef -D_MSC_VER=1900 and manual fixup of false matches. We also remove the predefined WIN32_VC* definitions as nobody is using them.

Support for MSVC below 2015 (aka vc14 aka _MSC_VER 1900) was removed in a previous commit.

We nowadays alwas mark Except::throw_exception as [[noreturn]] therefore all compiler should be able to figure out that the function does not need a return value here.

t-b · 2020-01-22T15:38:57Z

@mliszcz @bourtemb Rebased.

t-b requested review from bourtemb and mliszcz as code owners January 11, 2020 02:02

t-b mentioned this pull request Jan 17, 2020

Fix build on ppc 3 #657

Closed

bourtemb reviewed Jan 17, 2020

View reviewed changes

cppapi/client/api_util.cpp Show resolved Hide resolved

cppapi/client/devapi_attr.tpp Outdated Show resolved Hide resolved

cppapi/server/attrsetval.tpp Outdated Show resolved Hide resolved

cppapi/server/w_attrsetval.tpp Outdated Show resolved Hide resolved

t-b force-pushed the require-c++11-and-cmake-3.7 branch 2 times, most recently from b9479d8 to 1f8c2c8 Compare January 21, 2020 16:57

t-b mentioned this pull request Jan 21, 2020

Remove fallback snprintf implementation #659

Merged

t-b added the Ready For Merge label Jan 22, 2020

bourtemb reviewed Jan 22, 2020

View reviewed changes

.travis/debian8/Dockerfile Show resolved Hide resolved

mliszcz reviewed Jan 22, 2020

View reviewed changes

configure/CMakeLists.txt Show resolved Hide resolved

bourtemb previously approved these changes Jan 22, 2020

View reviewed changes

cppapi/server/tango_config.h Show resolved Hide resolved

.travis/debian8/Dockerfile Show resolved Hide resolved

t-b requested a review from mliszcz January 22, 2020 14:39

mliszcz previously approved these changes Jan 22, 2020

View reviewed changes

t-b added 9 commits January 22, 2020 16:33

CMakeLists.txt: Require C++11 and cmake 3.7

7158749

We don't use per target properties like cxx_std_11 as these are only supported starting with cmake 3.8. So we set it globally instead.

cppapi/server/tango_config.h: Remove test for gcc >= 3.0

c46ea53

This is way too old to work anymore as we now require C++11.

Remove HAS_UNIQUE_PTR

ce2cdab

We now require C++11. Done with unifdef -DHAS_UNIQUE_PTR

Remove HAS_MAP_AT

6d494e5

We now require C++11. Done with unifdef -DHAS_MAP_AT

Remove HAS_OVERRIDE

84b98e3

We now require C++11. Done with unifdef -DHAS_OVERRIDE

Remove HAS_RVALUE

7483d74

We now require C++11. Done with unifdef -DHAS_RVALUE

Remove HAS_THREAD

fea8ade

We now require C++11. Done with unifdef -DHAS_THREAD

Remove HAS_TYPE_TRAITS

1273f94

We now require C++11. Done with unifdef -DHAS_TYPE_TRAITS

Remove HAS_LAMBDA_FUNC

2c88f9c

We now require C++11. Done with unifdef -DHAS_LAMBDA_FUNC

t-b added 19 commits January 22, 2020 16:33

cppapi/server/tango_config.h: Map Tango_isnan always to std::isnan

94b3691

We now always require C++11.

Prefer nullptr over Tango_nullptr

dd5232a

We now require C++11 so nullptr is always available. But we keep the definitions of Tango_nullptr for backwards compatibility.

Remove HAS_RANGE_BASE_FOR

ccf6d3b

We now require C++11. Done with unifdef -DHAS_RANGE_BASE_FOR

Remove INIT_LIST

02b174c

We now require C++11. Done with unifdef -DINIT_LIST

Remove HAS_UNDERLYING

03bccb9

We now require C++11. Done with unifdef -DHAS_UNDERLYING

cppapi/server/tango_config.h: Map TANGO_NORETURN always to [[noreturn]]

893c56c

We now always require C++11.

cppapi/server/tango_config.h: Remove #ifdef spaghetti used for C++11 …

2b5f357

…feature checking We now always require C++11.

cppapi/client/ApiUtil.h: Remove unused _KillProc_ class

1cb8f1b

Travis: Install newer cmake on debian 8

69e5138

.travis: Remove testing for debian wheezy

2858602

This is just too old as it has only GCC 4.7.2.

cppapi/server/idl: Ignore unused-parameter for clang as well

5424da3

Exposed with the new C++11 compilation, but ultimately forgotten in cc8f943 (cppapi/server: Ignore warnings for generated code, 2019-11-15).

cppapi/client/eventconsumer.h: Add missing override keywords

64dc2a7

Found with -Winconsistent-missing-override using latest clang 9.0.

cppapi/server/tango.h: Simplify TANGO_UNUSED handling

33910b8

We don't support GCC < 4 anymore.

CMakeLists.txt: Unify and add more version output on configuring

a52d8b6

We now output cmake/compiler and platform versions for easier debugging. Output like the CMAKE_BUILD_TYPE was also changed to be outputted always regardless if default or not.

Remove conditional code for _MSC_VER < 1900

54adedc

Removed with unifdef -D_MSC_VER=1900 and manual fixup of false matches. We also remove the predefined WIN32_VC* definitions as nobody is using them.

log4tango: Remove fallback code for old MSVC versions

435aa9f

Support for MSVC below 2015 (aka vc14 aka _MSC_VER 1900) was removed in a previous commit.

cppapi/server/device.cpp: Remove compiler silencing return values

17abaed

We nowadays alwas mark Except::throw_exception as [[noreturn]] therefore all compiler should be able to figure out that the function does not need a return value here.

t-b dismissed stale reviews from mliszcz and bourtemb via 17abaed January 22, 2020 15:38

t-b force-pushed the require-c++11-and-cmake-3.7 branch from 100f45f to 17abaed Compare January 22, 2020 15:38

t-b requested review from bourtemb and mliszcz January 22, 2020 15:39

mliszcz approved these changes Jan 22, 2020

View reviewed changes

bourtemb approved these changes Jan 22, 2020

View reviewed changes

t-b merged commit 81286b2 into tango-controls:tango-9-lts Jan 22, 2020

t-b deleted the require-c++11-and-cmake-3.7 branch January 22, 2020 20:10

CMakeLists.txt: Require C++11 and cmake 3.7 #653

CMakeLists.txt: Require C++11 and cmake 3.7 #653

Uh oh!

Conversation

t-b commented Jan 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mliszcz commented Jan 11, 2020

Uh oh!

t-b commented Jan 14, 2020

Uh oh!

bourtemb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

t-b commented Jan 20, 2020

Uh oh!

t-b commented Jan 22, 2020

Uh oh!

Uh oh!

Uh oh!

bourtemb commented Jan 22, 2020

Uh oh!

t-b commented Jan 22, 2020

Uh oh!

bourtemb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

t-b commented Jan 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mliszcz commented Jan 22, 2020

Uh oh!

mliszcz left a comment

Choose a reason for hiding this comment

Uh oh!

t-b commented Jan 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

t-b commented Jan 11, 2020 •

edited

Loading

t-b commented Jan 22, 2020 •

edited

Loading