Skip to content

fix: address the final few compile warnings#2998

Open
bruno-dasilva wants to merge 2 commits into
beyond-all-reason:masterfrom
bruno-dasilva:bruno/fix-warnings-final
Open

fix: address the final few compile warnings#2998
bruno-dasilva wants to merge 2 commits into
beyond-all-reason:masterfrom
bruno-dasilva:bruno/fix-warnings-final

Conversation

@bruno-dasilva
Copy link
Copy Markdown
Collaborator

There's a final few compiler warnings that slipped through the earlier testing, as with before I will add comments to each with the warning printed.

AI Disclosure

I worked with claude code to help find the right way to address each warning.

@bruno-dasilva
Copy link
Copy Markdown
Collaborator Author

draft until I add the inline warnings as comments


# define the field splitter(-regex)
FS = "(,)|(\\()|(\\)\\;)";
FS = "(,)|(\\()|(\\);)";
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

all these warnings for \; were because there's no need to escape semicolons:

awk: /www/projects/RecoilEngine/AI/Wrappers/Cpp/bin/combine_wrappCallback.awk:270: warning: regexp escape
  sequence `\;' is not a known regexp operator


# define the field splitter(-regex)
FS = "(\\()|(\\)\\;)";
FS = "(\\()|(\\);)";
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

same as the other awk file, no need to escape semicolons

set(_skirmish_ai_gcc_warns
-Wno-maybe-uninitialized
-Wno-deprecated-declarations
-Wno-deprecated
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

windows build warning

  is deprecated [-Wdeprecated]
     55 |         __asm__ __volatile__ (
        |         ^~~~~~~
     note: the value of the stack pointer after an 'asm' statement must be the same as it was before the
  statement

-Wno-maybe-uninitialized
-Wno-deprecated-declarations
-Wno-deprecated
-Wno-sign-compare
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

oddly a warning that only triggers in arm builds 🤷 compilers be weird

.../as_callfunc_arm64.cpp:294:40: warning: comparison of integer expressions of different signedness:
  'const int' and 'long unsigned int' [-Wsign-compare]
    294 |   if( structSize <= sizeof(double) * 2 )
        |       ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~
  .../as_callfunc_arm64.cpp:313:52: warning: comparison of integer expressions of different signedness:
  'int' and 'long unsigned int' [-Wsign-compare]
    313 |   if( retType.GetSizeInMemoryBytes() > sizeof(asQWORD) )
        |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~

assert(f != 0.0f); // log10(0) = -inf

if (f < 1.0f || f >= (SPRING_INT64_MAX >> 1))
if (f < 1.0f || f >= static_cast<float>(SPRING_INT64_MAX >> 1))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

just missed this one earlier

/www/projects/RecoilEngine/rts/lib/lua/include/LuaUser.cpp:384:41: warning: implicit conversion from
  'long' to 'float' changes value from 4611686018427387903 to 4611686018427387904
  [-Wimplicit-const-int-float-conversion]
    384 |         if (f < 1.0f || f >= (SPRING_INT64_MAX >> 1))
        |                           ~~  ~~~~~~~~~~~~~~~~~^~~~~~
  1 warning generated.

Comment thread rts/lib/CMakeLists.txt
if(TARGET ${_mi_tgt})
target_compile_options(${_mi_tgt} PRIVATE -Wno-attributes -Wno-array-bounds)
endif()
endforeach()
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the new -Wno-array-bounds is for windows builds:

In file included from /usr/share/mingw-w64/include/winnt.h:27,
                   ...
                   from /build/src/rts/lib/mimalloc/src/alloc.c:13:
  In function '__readgsqword',
      inlined from 'NtCurrentTeb' at /usr/share/mingw-w64/include/winnt.h:10013:73,
      inlined from '__mi_prim_thread_id' at .../mimalloc/include/mimalloc/prim.h:287:21,
      inlined from '_mi_prim_thread_id' at .../mimalloc/include/mimalloc/prim.h:270:29,
      inlined from 'mi_free_ex' at .../mimalloc/src/free.c:175:31,
      inlined from 'mi_free' at .../mimalloc/src/free.c:198:3:
  /usr/share/mingw-w64/include/psdk_inc/intrin-impl.h:838:1: warning: array subscript 0 is outside array
  bounds of 'long long unsigned int[0]' [-Warray-bounds=]
    838 | __buildreadseg(__readgsqword, unsigned __int64, "gs", "q")
        | ^~~~~~~~~~~~~~
  In function 'mi_free':
  cc1: note: source object is likely at address zero

Comment thread rts/lib/CMakeLists.txt
Comment on lines +43 to +46
foreach(_mi_tgt mimalloc-static mimalloc-obj mimalloc)
if(TARGET ${_mi_tgt})
target_compile_options(${_mi_tgt} PRIVATE -Wno-attributes -Wno-array-bounds)
endif()
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we swap to using a loop over the three specific targets here, because previously the add_compile_options(-Wno-attributes) was needlessly hitting more things than just mimalloc. It's more precise of a suppression this way :)

Comment thread rts/lib/CMakeLists.txt
# Known GCC-only false positives in vendored RmlUi at -O2+:
# - -Warray-bounds (fixed in GCC 14 -- retest dropping it once CI is off GCC 13)
# - -Wstringop-overflow on Colourb::operator[] (walks contiguous rgba; GCC reads it as writing past red)
target_compile_options(rmlui_core PRIVATE -Wno-array-bounds -Wno-stringop-overflow)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

/build/src/rts/lib/RmlUi/Source/Core/PropertyParserColour.cpp: In static member function 'static bool
  Rml::PropertyParserColour::ParseHSLColour(Rml::Colourb&, const Rml::String&)':
  /build/src/rts/lib/RmlUi/Source/Core/PropertyParserColour.cpp:327:27: warning: writing 1 byte into a
  region of size 0 [-Wstringop-overflow=]
    327 |                 colour[i] = (byte)(Math::Clamp((int)(vals[i] * 255.0f), 0, 255));
        |                 ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  .../Include/RmlUi/Core/Colour.h:112:20: note: at offset 1 into destination object 'Rml::Colour<unsigned
  char, 255, false>::red' of size 1
    112 |         ColourType red, green, blue, alpha;
        |                    ^~~

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

perhaps there could be some sort of

union {
  struct { ColourType red, green, blue, alpha; };
  std::byte as_array [4];
}

Comment thread rts/System/MemPoolTypes.h
using TypesMem = std::aligned_storage_t< std::max({ sizeof(T)... }), std::max({ alignof(T)... }) >;
#if defined(__GNUC__)
#pragma GCC diagnostic pop
#endif
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this one I need to play around with in MSVC to see if we can find a better fix than just pragma'ing it. Will do that before merging.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There's some other MSVC build PR #2993

Comment thread CMakeLists.txt
if (IS_ARM64_BUILD)
# -Wpsabi just notes GCC 10.1's aarch64 aggregate-passing ABI change. That only
# matters when mixing pre/post-10.1 objects, which we never do. Drop the spam.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-psabi")
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

quite spammy in arm builds

In file included from /build/src/rts/System/Matrix44f.h:9,
                   from /build/src/rts/System/Matrix44f.cpp:3:
  /build/src/rts/System/float3.h: In member function 'std::pair<float3, float> float3::GetNormalized()
  const':
  /build/src/rts/System/float3.h:630:57: note: parameter passing for argument of type 'std::pair<float3,
  float>' when C++17 is enabled changed to match C++14 in GCC 10.1
    630 |         std::pair <float3, float> GetNormalized() const {
        |                                                         ^

@bruno-dasilva bruno-dasilva marked this pull request as ready for review June 2, 2026 19:19
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