Skip to content

Fix GCC 10-15 warnings (-Wall -Wextra), including several real bugs, upstream #421 and part of #260 - #6

Merged
jmgilbert2 merged 11 commits into
masterfrom
gcc-warnings
Oct 21, 2026
Merged

jmgilbert2 merged 11 commits into
masterfrom
gcc-warnings

Conversation

@jmgilbert2

@jmgilbert2 jmgilbert2 commented Oct 20, 2026 •

Copy link
Copy Markdown
Owner

Cleans up the warnings that GCC 10 to 15 report with -Wall -Wextra on EASTL's library and test sources. Several of them were real bugs. Addresses part of upstream electronicarts#260 and fixes upstream electronicarts#421 and PR electronicarts#595.

How it was checked

  • GCC: compile-only sweeps on GitHub Actions (the gcc-probe* scratch branches), GCC 10 to 15. Each covered library sources, test sources and repros, in six builds: -O0 Debug, -O1, -O2, -O3, -O2 with the April 2025 deprecations disabled, and -O2 -std=c++17.
    • EASTL's own warning sites went from 73 to the few listed under "Not fixed here".
    • Every test-code warning is gone.
    • Nothing new appeared in any build.
  • Windows: EASTL's test suite on MSVC and clang-cl × C++17/latest × Debug/Release. 8/8 passed, and the C++17 and latest outputs are identical.

Real bugs fixed

  • tuple_vector: variadicAnd(bool cond, bool conds...) was a C varargs function, so the iterator validation ignored every element type after the second. It's now a variadic template.
  • DecodePart (DecodePart pointer type punning breaks strict aliasing electronicarts/EASTL#421): the forwarding overloads accessed the callers' pointer variables through references to other pointer types, which breaks strict aliasing. They now convert the pointer values and write them back.
  • segmented_vector: begin() const and end() const returned an iterator reinterpreted as a const_iterator, and end() left two members uninitialized.
  • SetAssertionFailureFunction (PR Forward pContext to SetAssertionFailureFunction electronicarts/EASTL#595): the legacy overload dropped pContext.
  • Tests:
    • TestIntrusiveSDList printed c_str() of a destroyed temporary.
    • A precedence bug in TestSegmentedVector meant one check didn't test the values.

electronicarts#260: fixed_vector without overflow

EASTL_CRASH() now traps after its invalid write on GCC and clang, so the compiler knows the exhausted allocator never returns null. That clears the -Wnonnull warnings from fixed_string and vector growth. The assign()-past-capacity path stays (see below).

Warnings on correct code

  • Code changes:
    • a void* cast for -Wclass-memaccess;
    • braces for -Wempty-body;
    • EA_MAYBE_UNUSED parameters;
    • a named extract_key instead of a temporary, which GCC 13 took for a dangling reference;
    • an always-false trait instead of static_assert(I != I);
    • literal operators without the space that C++23 deprecates.
  • Suppressions: narrow, version-guarded suppressions for GCC false positives in inlined paths. Each covers one function or statement and says why the code is correct:
    • Compare;
    • CharStringUninitializedCopy;
    • shared_array::unique;
    • radix_sort's copy-back;
    • basic_string::erase;
    • fixed_vector_allocator::deallocate;
    • BitsetBase::set;
    • the two memmove helpers that vector::insert reaches.

Not fixed here

  • Left on purpose, because the only fixes cost a branch or change semantics:
    • GCC 10 to 15 report -Wnonnull at memory.h:319 when a fixed_vector without overflow is assigned past its capacity. Avoiding it would add a branch to every reallocation.
    • GCC 10 to 15 report -Wuninitialized/-Wmaybe-uninitialized in eastl::swap (utility.h:56) from string's Layout swap. Layout's move operations must keep swapping.
  • expected's constructors set mHasValue before constructing the value or error in the constructor body. If that throws, the base destructor destroys an object that was never constructed. GCC reports -Wmaybe-uninitialized on those exception paths in TestExpected (vector.h), plus a GCC 10-only one in the value-to-error assignment (smart_ptr.h:167). optional's copy/move path has the same order. Left alone: our builds disable exceptions.
  • smart_ptr_deleter<void> frees with delete[] (char*)p, which mismatches TestSmartPtr's scoped_ptr<void>(new int) (GCC: -Wmismatched-new-delete). Left alone by decision.
  • With the April 2025 deprecations disabled (legacy optional), GCC 13 to 15 report -Wmaybe-uninitialized in sort.h and optional.h for optional<int>.
  • EABase's typedef char char8_t; gives -Wc++20-compat in C++17 builds on GCC 13+; that's for EABase.
  • GCC 11 ignores the suppression pragmas for some inlined code, even at the warning's own location; GCC 12+ honors them. Two of the false positives above stay on GCC 11 only:
    • Compare (-Wstringop-overread);
    • the fixed_vector deallocation, at -O3 (-Wfree-nonheap-object).
  • Only EASTL's sources, and the headers of the other packages that its tests include, were compiled with GCC; the other EA repos weren't.

🤖 Generated with Claude Code

@jmgilbert2 jmgilbert2 changed the title Fix GCC 10-15 warnings (-Wall -Wextra), including several real bugs and upstream #260/#421 Fix GCC 10-15 warnings (-Wall -Wextra), including several real bugs, upstream #421 and part of #260 Oct 21, 2026
jmgilbert2 and others added 11 commits October 21, 2026 00:30
…tronicarts#260)

The fixed_vector allocator without overflow calls EASTL_CRASH() and then
returns NULL. GCC can't tell that the crash doesn't return, so after
inlining it sees vector and string code memmove into that NULL and warns
with -Wnonnull. Trap after the invalid write so the compiler knows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…asserts

variadicAnd(bool cond, bool conds...) was a C varargs function, so it only
checked its first two arguments: tuple_vector's iterator validation
ignored every element type after the second. It's now a variadic template.

The index-out-of-range static_asserts in tuple and tuple_vector used
I != I, which GCC 14 reports with -Wtautological-compare. They use an
always-false trait that depends on I instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… types (upstream electronicarts#421)

The forwarding overloads reinterpret_cast the callers' pointer variables
(for example const char8_t*& to const char*&) and write through them,
which breaks strict aliasing; GCC reports it with -Wstrict-aliasing. They
now go through details::DecodePartAs, which converts the pointer values
and writes them back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alized members

begin() const and end() const filled an iterator and returned it as a
const_iterator&, a different type, and end() left mEnd and mSegment
uninitialized (GCC: -Wstrict-aliasing, -Wuninitialized). They now build a
value-initialized const_iterator, and the non-const versions value-
initialize their iterator too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
operator"" sv with a space is deprecated since C++23 (CWG 2521); GCC 15
reports it with -Wdeprecated-literal-operator. The suffixes stay inside
the existing reserved-suffix warning suppressions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…upstream PR electronicarts#595)

The overload taking an EASTL_AssertionFailureFunction set the function
but dropped pContext, so the user's function got a null context. It now
goes through the EASTL_AssertionFailureFunctionEx overload, which stores
the context. GCC reported the dropped parameter with -Wunused-parameter.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- memory.h: cast the memmove destination to void*, as GCC documents for
  -Wclass-memaccess; the types are trivially copyable.
- vector.h: brace the if constexpr whose body is an assert that may be
  empty (-Wempty-body).
- utility.h, variant.h: mark parameters that are unused for some template
  arguments or configurations EA_MAYBE_UNUSED (-Wunused-parameter and
  -Wunused-but-set-parameter).
- red_black_tree.h, map.h: call a named extract_key instead of a
  temporary, which GCC 13 takes for a dangling reference
  (-Wdangling-reference).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… GCC

- TestIntrusiveSDList: the failure messages printed c_str() of a string
  temporary that was already destroyed (-Wdangling-pointer).
- TestSegmentedVector: *it == (i < 55) ? 0 : 5 parsed as
  (*it == (i < 55)) ? 0 : 5, so the check didn't test the values
  (-Wint-in-bool-context).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Mark results that are only there to check that a call compiles, and
  typedefs of a local iterator class, EA_MAYBE_UNUSED
  (-Wunused-but-set-variable, -Wunused-local-typedefs).
- Leave unused lambda and visitor parameters unnamed (-Wunused-parameter).
- Use size_t loop counters against size_t bounds (-Wsign-compare).
- Parenthesize && inside || (-Wparentheses).
- Initialize enable_shared_from_this explicitly in user copy constructors
  (-Wextra).
- Use a named visitor, which GCC 13 doesn't take for a dangling
  reference (-Wdangling-reference).
- Name the namespace of TestIntrusiveHash's types: with an anonymous one,
  the explicit instantiations leave unused internal-linkage member
  functions (-Wunused-function).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each suppression covers one function or statement, says why GCC is wrong
there, and is limited to the GCC versions whose warning name exists:

- char_traits.h Compare(const char*): -Wstringop-overread (GCC 11-14) and
  -Wstringop-overflow (GCC 10), bounds the callers have ruled out.
- char_traits.h CharStringUninitializedCopy: -Wstringop-overread (GCC 12-15)
  on reallocation paths that need a longer source than the literal.
- shared_array::unique: -Wuse-after-free (GCC 12) after copy-and-swap.
- radix_sort's final copy: -Wstringop-overflow (GCC 12-15, -O3).
- basic_string::erase: -Wstringop-overflow (GCC 10, -O3).
- fixed_vector_allocator::deallocate: -Wfree-nonheap-object (GCC 12-15);
  it never frees the fixed buffer.
- BitsetBase::set: -Warray-bounds and -Wstringop-overflow (GCC 11, -O3);
  bitset::set checks i < N.

GCC 11 ignores these pragmas for some inlined code, so it still reports
Compare and, at -O3, the fixed_vector deallocation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GCC 10 to 15 report null pointers in copy's and move_backward's memmove
specializations when vector::insert inserts into an empty vector: they
come from the branch for ranges that fit in the capacity, which a
non-empty range can't take with a zero capacity. GCC warns before it
folds the condition, so restructuring vector::insert didn't help.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jmgilbert2
jmgilbert2 merged commit eb5a612 into master Oct 21, 2026
3 of 15 checks passed
jmgilbert2 added a commit that referenced this pull request Oct 21, 2026
Brings PR #6 (GCC 10-15 warning cleanup) and PR #7 (uninitialized_fill
copy-constructs unless the type is trivially copyable, upstream electronicarts#485).
PR #8 only changed the benchmark and test projects and the test
submodule pins, which don't apply to this branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant