fix(build): stop GCC's optimizer-invented warnings failing the build - #7699
Merged
Conversation
FindSmallestEdge is a virtual on the CH datafacade, so the callable it takes has to be type erased. std::function was doing that with a small-buffer union it never needed: all four call sites hand it a stateless lambda, so nothing is ever stored. Replace it with util::FunctionRef, a non-owning object pointer next to a plain function pointer. No union, no allocation, and the call cannot throw. This also settles a GCC 15 build failure. GCC could not prove that std::function's _Any_data union was live on the path it inlined through unpackPath, and reported it from lto1 as a maybe-uninitialized read. FunctionRef has no union, so the construct GCC objected to is gone rather than merely inlined differently.
Building master on Arch with GCC 15.2.1 fails under -Werror, and fails differently depending on how it is configured. The LTO release trips maybe-uninitialized inside std::function, the non-LTO release trips stringop-overread on a strlen libosmium inlines, and the reporter of #7690 trips maybe-uninitialized inside std::variant's visit-based move. None of the three is reachable code, and no one of them is the same line as the others. CI never saw any of this because the gcc-14 and gcc-15 jobs were passing -Wno-array-bounds -Wno-uninitialized through CXXFLAGS, added back when the GCC 12 job landed and copied forward since. That suppression lived in the workflow, so it covered CI and nobody else. Move it into the build, and narrow it. warnings.cmake now disables maybe-uninitialized for GNU, next to the stringop-overflow entry that is already there for the same reason. -Wuninitialized stays enabled, so the definite case is still an error, which is stricter than what CI has been running since 2022. Neither -Wno-array-bounds nor -Wno-uninitialized turned out to be needed, so both come out of the workflow. The libosmium overread is suppressed at its include instead, since it has a source file to attach a pragma to. The maybe-uninitialized ones do not: under -flto they are reported from lto1 against an ltrans object, with no translation unit to scope a pragma to. Fixes #7690
…tion scripts/format.sh has wanted clang-format-22 for a while, but AGENTS.md still asked for 15. Also record why cmake/warnings.cmake disables a few GCC warning families outright, so the next reader does not take the "fix them, don't ignore them" rule as licence to delete the suppressions and reopen #7690.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7699 +/- ##
==========================================
+ Coverage 94.73% 94.76% +0.02%
==========================================
Files 524 526 +2
Lines 41723 41778 +55
==========================================
+ Hits 39526 39590 +64
+ Misses 2197 2188 -9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…st it FunctionRef accepted a plain function, because is_invocable_r is happy with one, and then failed inside the constructor where taking a void* to a function pointer is ill-formed. The caller got a page of template errors from a header they did not write. Constrain on is_object_v so a plain function is rejected during overload resolution instead, while lambdas, function objects and function pointers all still bind. Add the unit tests the header should have come with. They pin the reference semantics that make it different from std::function, that arguments reach the callable without a copy, the two-pointer layout the GCC fix depends on, and which callables the constraint admits. Also say plainly in the comment that a FunctionRef must not be stored. Binding one to a temporary and keeping it compiles silently, and the type cannot prevent that.
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.
Issue
Fixes #7690.
Master does not build on Arch with GCC 15.2.1, and it fails differently depending on how it is configured. Reproducing it in a pinned Arch container turned up three separate
-Werrorfailures, no two of them on the same line:maybe-uninitializedgeojson_debug_policy_toolkit.hpp:62, viastd::variant's visit-based movemaybe-uninitializedrouting_base_ch.hpp:222, viastd::function, reported fromlto1stringop-overreadosm_object_builder.hpp:500, inlined intoarea_manager.cpp:204All three are code paths the optimizer created rather than anything reachable, and which one fires first moves with the GCC build and the
-fltosetting.CI never saw any of it. The
gcc-14andgcc-15jobs were passing-Wno-array-bounds -Wno-uninitializedthroughCXXFLAGS, added when the GCC 12 job landed in #6455 and copied forward ever since.-Wno-uninitializedalso disables-Wmaybe-uninitialized, so those jobs were green only because the workflow suppressed for itself what nobody building from source ever got.What this changes
A real fix where there was one.
FindSmallestEdgetook astd::function, and GCC's complaint named_Any_data, its small-buffer union. All four call sites pass stateless lambdas, so the erasure stored nothing.util::FunctionRefreplaces it with an object pointer next to a function pointer: no union, no allocation, no throwing call. The construct GCC objected to is gone rather than inlined differently.A scoped pragma where the code is not ours. The libosmium overread is suppressed at its
#include. It needs a__GNUC__ && !__clang__guard, because clang rejects-Wstringop-overreadas an unknown warning group and would fail every clang job under-Werror.One flag, moved into the build and narrowed.
cmake/warnings.cmakedisablesmaybe-uninitializedfor GNU, beside thestringop-overflowentry already there for the same reason. This is the only change that covers the reported failure, which I could not reproduce and therefore cannot fix at the source. Under-fltothese are reported fromlto1against an ltrans object, so there is no translation unit to scope a pragma to.-Wuninitializedstays enabled, so the definite case is still an error. Neither-Wno-array-boundsnor-Wno-uninitializedturned out to be needed, so both come out of the workflow and CI now builds what users build. Net, this is stricter than what CI has been running since 2022.Verification
Arch container pinned to the archive snapshot carrying GCC 15.2.1, dependencies via vcpkg as documented:
-Werroron throughoutlibrary-testsrouting on monaco through the CH unpacking path this touchesFunctionRefchange alone fixes the LTO failure with no cmake suppression active, confirmed against the generated compile lineCaveats
I could not reproduce the reported
std::variantfailure in 18 configurations across GCC 14.4, 15.3 and 16.2, at-O2and-O3, with and without LTO, nor under_FORTIFY_SOURCE=3,_GLIBCXX_ASSERTIONSor-march=nativeon 15.2.1 itself. Only that specific Arch rebuild produces it, so the retained flag is what closes the issue for the reporter. A source-level fix there would be either an unverifiable one-line change or takingjson::Valueoffstd::variant, which is a refactor of the whole JSON and API layer.Validation was on Arch only. Dropping the workflow
CXXFLAGS, in particular-Wno-array-boundson Ubuntugcc-14, is the part CI itself has to confirm. If it still trips, the answer is anotherno_warning(array-bounds)line rather than restoring the workflow hack.AI participation: 🤖 Claude Code, Claude Opus 5.
Tasklist
Requirements / Relations
Fixes #7690. Reverts the CI workaround introduced in #6455 and carried forward through #6865, #6905 and #7574. Same warning family as #7422, which had a source-level fix available.