Skip to content

Fixed #2272: Compilation failure with C++20#2297

Merged
horenmar merged 3 commits intocatchorg:v2.xfrom
codeinred:v2.x
Oct 4, 2021
Merged

Fixed #2272: Compilation failure with C++20#2297
horenmar merged 3 commits intocatchorg:v2.xfrom
codeinred:v2.x

Conversation

@codeinred
Copy link
Copy Markdown
Contributor

@codeinred codeinred commented Oct 3, 2021

Description

This commit fixes issue #2272: Compilation failure with C++20.

To do this, it removes using-declarations for

  • std::allocator<T>::address(),
  • std::allocator<T>::construct(),
  • std::allocator<T>::max_size(), and
  • std::allocator<T>::destroy()

from the CustomAllocator class in Matchers.tests.cpp. These functions were deprecated in C++17, and removed in C++20, so having a using declaration for them causes compilation error. The functions weren't used in the Catch2 code base in earlier versions of C++, so it's not a breaking change.

This commit also wraps the use of a comma operator inside the bracket operator in Message.tests.cpp. This change is necessary because gcc was issuing an error when compiling tests with -std=c++20.

CAPTURE(std::vector<int>{1, 2, 3}[0, 1, 2],

Becomes

CAPTURE(std::vector<int>{1, 2, 3}[(0, 1, 2)],
Click to see compilation error
projects/CMakeFiles/SelfTest.dir/SelfTest/UsageTests/Message.tests.cpp.o 
/usr/bin/c++  -I/home/alecto/cowork/Catch2/include -std=c++20 -Wall -Wextra -Wunreachable-code -Wpedantic -Wmissing-declarations -Werror -MD -MT projects/CMakeFiles/SelfTest.dir/SelfTest/UsageTests/Message.tests.cpp.o -MF projects/CMakeFiles/SelfTest.dir/SelfTest/UsageTests/Message.tests.cpp.o.d -o projects/CMakeFiles/SelfTest.dir/SelfTest/UsageTests/Message.tests.cpp.o -c /home/alecto/cowork/Catch2/projects/SelfTest/UsageTests/Message.tests.cpp
In file included from /home/alecto/cowork/Catch2/include/catch.hpp:54,
                 from /home/alecto/cowork/Catch2/projects/SelfTest/UsageTests/Message.tests.cpp:9:
/home/alecto/cowork/Catch2/projects/SelfTest/UsageTests/Message.tests.cpp: In function ‘void ____C_A_T_C_H____T_E_S_T____76()’:
/home/alecto/cowork/Catch2/projects/SelfTest/UsageTests/Message.tests.cpp:245:40: error: top-level comma expression in array subscript is deprecated [-Werror=comma-subscript]
  245 |     CAPTURE(std::vector<int>{1, 2, 3}[0, 1, 2],
      |                                        ^
cc1plus: all warnings being treated as errors

This commit also ran scripts/approve.py, which updated compact.sw.approved.txt, console.sw.approved.txt, and xml.sw.approved.txt to reflect the change made in Message.tests.cpp.

GitHub Issues

This PR was motivated by #2272

This commit fixes issue catchorg#2272: Compilation failure with C++20. To
do this, it removes using-declarations for deprecated functions
std::allocator<T>::address(), construct(), max_size(), and destroy()
from the CustomAllocator class in Matchers.tests.cpp; and it
wraps the use of a comma operator inside the bracket operator in
Message.tests.cpp. This commit also updates compact.sw.approved.txt,
console.sw.approved.txt, and xml.sw.approved.txt to reflect the change
made in Message.tests.cpp.

Signed-off-by: Alecto Irene Perez <perez.cs@pm.me>
Ah, MSVC, my old nemesis, we meet again. This commit amends the
previous commit to re-introduce the following using declarations inside
the CustomAllocator class:

- using std::allocator<T>::address;
- using std::allocator<T>::construct;
- using std::allocator<T>::max_size;
- using std::allocator<T>::destroy;

When building with MSVC and using a C++ standard older than
C++17. These were re-introduced because MSVC throws an error if they're
not present when compiling with C++11 or C++14.

Signed-off-by: Alecto Irene Perez <perez.cs@pm.me>
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 3, 2021

Codecov Report

Merging #2297 (5da925c) into v2.x (3d01f3a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             v2.x    #2297   +/-   ##
=======================================
  Coverage   90.09%   90.09%           
=======================================
  Files         113      113           
  Lines        5035     5035           
=======================================
  Hits         4536     4536           
  Misses        499      499           

Copy link
Copy Markdown
Member

@horenmar horenmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but the fixes should be done a bit differently (see comments)

This commit restores the previous state of 'CAPTURE can deal with
complex expressions involving commas', and instead uses localized
warning suppression to disable messages warnings about the use of a
comma inside a bracket operator.

Signed-off-by: Alecto Irene Perez <perez.cs@pm.me>
@codeinred
Copy link
Copy Markdown
Contributor Author

codeinred commented Oct 3, 2021

This should be instead handled by a localized warning suppression instead. The point of the test is to check that CAPTURE handles various weird expressions properly, including commas directly in subscript operator.

I added localized warning suppression and restored the initial state of the test!

The deprecated ones should be safe to just delete afaik.

That was my initial assumption too, but this broke in MSVC 2015, which checked for the existence of those methods. Since these methods become deprecated in C++17, I added a #if to check for that.

@codeinred codeinred requested a review from horenmar October 4, 2021 00:27
@horenmar horenmar merged commit dba29b6 into catchorg:v2.x Oct 4, 2021
@horenmar
Copy link
Copy Markdown
Member

horenmar commented Oct 4, 2021

Thanks

codeinred added a commit to codeinred/Catch2 that referenced this pull request Oct 11, 2021
It turns out that Issue catchorg#2272 partially affected the devel branch. When
building tests with C++20, the compiler emits a warning that top-level
comma expressions in array subscripts are depricated. Warnings are
treated as errors, so this caused the build to fail.

This commit adds localized warning suppression
in accordance with this recommendation here:
catchorg#2297 (comment)

Signed-off-by: Alecto Irene Perez <perez.cs@pm.me>
horenmar pushed a commit that referenced this pull request Oct 21, 2021
* Apply PR #2297 to devel branch

It turns out that Issue #2272 partially affected the devel branch. When
building tests with C++20, the compiler emits a warning that top-level
comma expressions in array subscripts are depricated. Warnings are
treated as errors, so this caused the build to fail.

This commit adds localized warning suppression
in accordance with this recommendation here:
#2297 (comment)

Signed-off-by: Alecto Irene Perez <perez.cs@pm.me>

* Fixed unknown pragma warning on old versions of gcc & clang

This commit fixes an unkwown pragma warning on older versions of GCC
and Clang. These older versions don't have a warning for depricated use
of the comma subscript. Because warning suppression is localized, and
only refers to the comma subscript warning, it doesn't affect compiler
warnings in other parts of the code.

Signed-off-by: Alecto Irene Perez <perez.cs@pm.me>

* More #warning backwards compatibility fixes

Signed-off-by: Alecto Irene Perez <perez.cs@pm.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants