Repository navigation
Fix -Wextra and -Wpedantic warnings in the headers - #2373
Merged
Merged
Conversation
ericniebler
approved these changes
Oct 30, 2026
ericniebler
force-pushed
the
fix-header-warnings
branch
from
October 30, 2026 03:36
55b3821 to
a3eee61
Compare
Collaborator
|
/ok to test a3eee61 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1116.
What
The tests build with
-Wall -Wextra -Wpedantic -Werror, but CMake adds the headers asSYSTEM, so the tests never see warnings inside them. Users who include the headers with a plain-Istill get those warnings, for example through meson'sinclude_directories('include')withwarning_level=3. On main, building the tests with the headers through-Igives 23 warning sites with GCC 12 and 9 with Clang 21. This PR fixes them:;after member function definitions, in 16 files (GCC-Wpedantic).-Wmissing-field-initializers(GCC and Clang):__continues_on's__state_base::__data_and__finally's__opstate_base::__result_get a{}default member initializer. Both structs are only built by aggregate initialization that leaves this member out, so it was already initialized from{}. Behaviour does not change.-Wunused-local-typedef(s): an unusedshape_talias inthread_pool_base.hpp.-Wmissing-bracesin__when_all.hpp: ignored for the file after__prologue.hpp, as__bulk.hpp,__just.hppand__tuple.hppalready do.-Wunused-valuein__receiver_ref.hpp: ignored for the file the same way, asexec/repeat_n.hppdoes. The left operand of the comma in__indirect_receiver::set_nexthas no effect, but it stays, because it is what makes the call depend on_Item.In
thread_pool_base.hpp,_bulk_sender::connectends withSTDEXEC_AUTO_RETURN(...). Without the;, clang-format treats the next member as part ofconnect, soconnectand the next member are now inside// clang-format off/on, as inbasic_sequence.hpp.Not changed: GCC
-Wsubobject-linkageat 4 places in__any.hpp. GCC reports it when the tests instantiate those templates with types from an anonymous namespace.Verification
Docker (Debian 12), GCC 12.2 and Clang 21.1 with libstdc++ 12, Debug, the tests' own flags, with the headers through
-Iinstead of-isystem. The GCC build also had-Wno-error=subobject-linkage.With this PR,
test.stdexecandtest.execbuild with-Werrorand pass with both compilers. That is 641 and 454 test cases with Clang. GCC runs one fewer intest.stdexec, because one test is skipped on GCC.Each header in
include/stdexecandinclude/execwas also compiled on its own with both compilers (-fsyntax-only -Wall -Wextra -Wpedantic, through-I). This skipped the nvexec, asio, tbb, taskflow, libdispatch and Windows headers. Only the deprecation#warnings inexec/repeat_effect_until.hppandexec/system_context.hppremain. clang-format 21 is clean.Not tested: MSVC, nvc++, Release builds, and the other CI configurations.
These warnings came back because the tests cannot see them. If you want, I can follow up with a change that builds the tests with the headers as non-system includes, so CI catches new ones. GCC would need
-Wno-subobject-linkagefor that.AI-usage disclosure: Claude (Anthropic) was used as assistant.