Skip to content

Fix Clang 19 -Wc++20-extensions warning#2968

Merged
ChrisThrasher merged 1 commit intocatchorg:develfrom
archie2x:feature/2910-fix-Wc++20-extensions-warnerr
Apr 7, 2025
Merged

Fix Clang 19 -Wc++20-extensions warning#2968
ChrisThrasher merged 1 commit intocatchorg:develfrom
archie2x:feature/2910-fix-Wc++20-extensions-warnerr

Conversation

@archie2x
Copy link

@archie2x archie2x commented Mar 28, 2025 •
edited
Loading

Description

  1. IInitial work (commit 5) removes WS/unrelated changes from PR Fix: Clang 19 -Wc++20-extensions warning #2910  #2934 meant to address issue Clang 19 -Wc++20-extensions warning #2910.
  2. commit 6 discards changes from PR Fix: Clang 19 -Wc++20-extensions warning #2910  #2934 in favor of correct clang-version specific warning suppression pragmas for CATCH_INTERNAL_SUPPRESS_ZERO_VARIADIC_WARNINGS

GitHub Issues

#2910

Minimal reproduction

Tested with:

// test.cpp
#include <catch2/catch_template_test_macros.hpp>
#include <catch2/catch_test_macros.hpp>

#include <vector>

TEMPLATE_TEST_CASE( "one-arg", "[bar]", int)
{
	std::vector<TestType> vec;
	vec.push_back(2);
}
> clang-${VERSION} -I Catch2/src -I ${GENERATED_INCLUDES} -Wall -Wextra -Wpedantic -Werror -c test.cpp

Patch Results

Catch2 Ubuntu clang-19 Ubuntu clang-20 OSX clang-19
devel fail success fail
patch success success success

Error seen (clang-19 only):
test.cpp:6:1: error: passing no argument for the '...' parameter of a variadic macro is a C++20 extension [-Werror,-Wc++20-extensions]

@archie2x archie2x mentioned this pull request Mar 28, 2025
Closed
@ChrisThrasher
Copy link
Collaborator

ChrisThrasher commented Mar 28, 2025

I'm still seeing the same problem present in #2934 as well as this implementation.

@ChrisThrasher ChrisThrasher changed the title Issue #2910 fix wc++20 extensions warnerr Fix Clang 19 -Wc++20-extensions warning Mar 28, 2025
@archie2x
Copy link
Author

archie2x commented Mar 28, 2025

In my tests, (osx/linux, arm64) this seemed to cause other problems.

For anybody w/ cmake I'm going with the following and moving on:

if(CMAKE_CXX_STANDARD EQUAL 17
   AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang"
   AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "19.0.0")
  # Fix for https://github.com/catchorg/Catch2/issues/2910 TEMPLATE_TEST_CASE()
  # "error: passing no argument for the '...' parameter of a variadic macro is a
  # C++20 extension [-Werror,-Wc++20-extensions]"
  target_compile_options(Catch2 INTERFACE -Wno-c++20-extensions)
endif()

@archie2x
Copy link
Author

archie2x commented Mar 30, 2025

Pretty sure commit 6 is the right thing to do here. Test clang-{19,20} for simple:

#include <catch2/catch_template_test_macros.hpp>
#include <catch2/catch_test_macros.hpp>

#include <vector>

TEMPLATE_TEST_CASE( "one-arg", "[bar]", int)
{
	std::vector<TestType> vec;
	vec.push_back(2);
}

TEMPLATE_TEST_CASE("two-arg", "[bar]", int, float) {
  std::vector<TestType> vec;
  vec.push_back(2);
}

@ChrisThrasher ChrisThrasher requested a review from horenmar March 30, 2025 15:05
@codecov
Copy link

codecov bot commented Mar 30, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.98%. Comparing base (76f70b1) to head (998dda5).
Report is 1 commits behind head on devel.

Additional details and impacted files 
@@           Coverage Diff           @@
##            devel    #2968   +/-   ##
=======================================
  Coverage   90.98%   90.98%           
=======================================
  Files         198      198           
  Lines        8599     8599           
=======================================
  Hits         7823     7823           
  Misses        776      776
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

horenmar
horenmar approved these changes Apr 1, 2025
View reviewed changes
Copy link
Member

@horenmar horenmar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Ugh, the differences in the warning name are annoying.

The changes seem fine.

@ChrisThrasher If you want me to take a second look at something, you need to ping me on Discord. I have about 100 notifications from Catch2 on GH, and I am currently ignoring them all.

@archie2x
Copy link
Author

archie2x commented Apr 7, 2025

Merge?

@ChrisThrasher ChrisThrasher force-pushed the feature/2910-fix-Wc++20-extensions-warnerr branch from 5bb8d12 to 998dda5 Compare April 7, 2025 20:59
ChrisThrasher
ChrisThrasher approved these changes Apr 7, 2025
View reviewed changes
Copy link
Collaborator

@ChrisThrasher ChrisThrasher left a comment

Choose a reason for hiding this comment

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

I was able to test this with Catch 19 and 20 and confirm that it works. Thanks!

@ChrisThrasher ChrisThrasher merged commit f51dc98 into catchorg:devel Apr 7, 2025
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@horenmar horenmar horenmar approved these changes

@ChrisThrasher ChrisThrasher ChrisThrasher approved these changes

+1 more reviewer

@shahsb shahsb shahsb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@archie2x @ChrisThrasher @horenmar @shahsb @abhishekbelgaonkar23