Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch2 not fully compliant with c++17 #2021

Closed
tdegeus opened this issue Sep 10, 2020 · 11 comments
Closed

Catch2 not fully compliant with c++17 #2021

tdegeus opened this issue Sep 10, 2020 · 11 comments

Comments

@tdegeus
Copy link
Contributor

tdegeus commented Sep 10, 2020

I'm trying to upgrade the conda-forge feedstock to the latest release (conda-forge/catch2-feedstock#33 ). However, I'm getting a build error:

../include/internal/catch_uncaught_exceptions.cpp: In function 'bool Catch::uncaught_exceptions()':
1023../include/internal/catch_uncaught_exceptions.cpp:20:21: error: 'bool std::uncaught_exception()' is deprecated [-Werror=deprecated-declarations]
1024         return std::uncaught_exception();
1025                     ^~~~~~~~~~~~~~~~~~
1026In file included from ../include/internal/catch_uncaught_exceptions.cpp:11:
1027$BUILD_PREFIX/powerpc64le-conda-linux-gnu/include/c++/8.4.0/exception:102:8: note: declared here
1028   bool uncaught_exception() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__pure__));
1029        ^~~~~~~~~~~~~~~~~~
1030../include/internal/catch_uncaught_exceptions.cpp:20:40: error: 'bool std::uncaught_exception()' is deprecated [-Werror=deprecated-declarations]
1031         return std::uncaught_exception();
1032                                        ^
1033In file included from ../include/internal/catch_uncaught_exceptions.cpp:11:
1034$BUILD_PREFIX/powerpc64le-conda-linux-gnu/include/c++/8.4.0/exception:102:8: note: declared here
1035   bool uncaught_exception() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__pure__));
1036        ^~~~~~~~~~~~~~~~~~
1037../include/internal/catch_uncaught_exceptions.cpp:20:40: error: 'bool std::uncaught_exception()' is deprecated [-Werror=deprecated-declarations]
1038         return std::uncaught_exception();
1039                                        ^
1040In file included from ../include/internal/catch_uncaught_exceptions.cpp:11:
1041$BUILD_PREFIX/powerpc64le-conda-linux-gnu/include/c++/8.4.0/exception:102:8: note: declared here
1042   bool uncaught_exception() _GLIBCXX_USE_NOEXCEPT __attribute__ ((__pure__));
1043        ^~~~~~~~~~~~~~~~~~
1044cc1plus: all warnings being treated as errors
@mjscosta
Copy link

Adding full build log:
https://travis-ci.org/github/conda-forge/catch2-feedstock/builds/725973219

Check here, Catch2 seems not be completely compliant with c++17:
https://travis-ci.org/github/conda-forge/catch2-feedstock/builds/725973219#L1018

resulting on the error posted by tdgeus.

Please advise, is Catch2 intended to be strict compliant with c++17?

@mjscosta
Copy link

Hi @tdegeus, could you change the issue name to something like: "Catch2 not fully compliant with c++17 causing tests to fail", I cannot since I'm not the author of the issue.

Thanks, Best.

@mjscosta
Copy link

Adding reference to conda-forge issue:
conda-forge/catch2-feedstock#32

@mjscosta
Copy link

The error seems to be related to the gcc version,

In the ppc environment the gcc version is 8.4
https://api.travis-ci.org/v3/job/726135670/log.txt

In the linux version the gcc version is 7.5
https://dev.azure.com/conda-forge/84710dde-1620-425b-80d0-4cf5baca359d/_apis/build/builds/207695/logs/17

According to https://en.cppreference.com/w/cpp/error/uncaught_exception, is deprecated in c++17, and gcc 8.4 already implemented the deprecation warning.

Since testes are running with option
// Enable all warnings as errors
CATCH_ENABLE_WERROR:BOOL=ON
This results in failed tests when using gcc 8.4

@tdegeus tdegeus changed the title Build error when trying to upgrade to the latest version on conda Catch2 not fully compliant with c++17 Sep 11, 2020
@horenmar
Copy link
Member

Right, so the problem is that the detection of stdlib's support for std::uncaught_exceptions is not firing, even though the stdlib has deprecated std::uncaught_exception, and thus we can surmise that std::uncaught_exceptions is indeed supported.

I think I know why it misfires, and how to fix it, but it will have to wait until I have the time.

@giellamo
Copy link

@mjscosta Am I wrong or using deprecated things is allowed by the standard?? So even if you use a deprecated thing you are compliant.

@mjscosta
Copy link

I'm not sure it is an issue regarding the standard or not, the issue I've got was on running the tests for Catch2, and the tests started to fail when using updated compiler (gcc 8.4), where the combination CATCH_ENABLE_WERROR:BOOL=ON (default as ON on the cmake), resulted on the tests failing.

I've fixed the issue by setting the option to OFF for packaging purposes. So the issue maybe should be addressed regarding the tests, as they will start to fail when you update the compiler to one fully supporting c++17, at least for gcc 8.4 >= .

@mjscosta
Copy link

Checking the builds, it seems that when executing the tests for COMPILER='g++-8' (https://travis-ci.org/github/catchorg/Catch2/jobs/726786262)
, the compiler version displayed on the job is 5.04:
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.12) 5.4.0 20160609

@horenmar
Copy link
Member

@mjscosta https://travis-ci.org/github/catchorg/Catch2/jobs/726786262#L387

@giellamo It is indeed allowed, just not recommended. Ironically we (wg21) still haven't figured out whether deprecated means "will be removed at some point", or just "you really shouldn't use this".

@mjscosta
Copy link

mjscosta commented Sep 15, 2020

https://en.cppreference.com/w/cpp/language/attributes/deprecated and according to https://en.cppreference.com/w/cpp/error/uncaught_exception, it will be removed in c++20.

@horenmar any idea why the error its occurring in the conda-forge packaging ?

@horenmar
Copy link
Member

horenmar commented Oct 6, 2020

Should be fixed in master.

horenmar added a commit that referenced this issue Oct 8, 2020
The problem was that Catch2 did not reliably include `<exception>`
before it checked for the feature test macro for
`std::uncaught_exceptions`. To avoid overhead of including
`<exception>` everywhere, the configuration check was split out
into a separate header.

Closes #2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants