fix: address the final few compile warnings#2998
Conversation
|
draft until I add the inline warnings as comments |
|
|
||
| # define the field splitter(-regex) | ||
| FS = "(,)|(\\()|(\\)\\;)"; | ||
| FS = "(,)|(\\()|(\\);)"; |
There was a problem hiding this comment.
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 = "(\\()|(\\);)"; |
There was a problem hiding this comment.
same as the other awk file, no need to escape semicolons
| set(_skirmish_ai_gcc_warns | ||
| -Wno-maybe-uninitialized | ||
| -Wno-deprecated-declarations | ||
| -Wno-deprecated |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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.
| if(TARGET ${_mi_tgt}) | ||
| target_compile_options(${_mi_tgt} PRIVATE -Wno-attributes -Wno-array-bounds) | ||
| endif() | ||
| endforeach() |
There was a problem hiding this comment.
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
| foreach(_mi_tgt mimalloc-static mimalloc-obj mimalloc) | ||
| if(TARGET ${_mi_tgt}) | ||
| target_compile_options(${_mi_tgt} PRIVATE -Wno-attributes -Wno-array-bounds) | ||
| endif() |
There was a problem hiding this comment.
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 :)
| # 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) |
There was a problem hiding this comment.
/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;
| ^~~
There was a problem hiding this comment.
perhaps there could be some sort of
union {
struct { ColourType red, green, blue, alpha; };
std::byte as_array [4];
}| using TypesMem = std::aligned_storage_t< std::max({ sizeof(T)... }), std::max({ alignof(T)... }) >; | ||
| #if defined(__GNUC__) | ||
| #pragma GCC diagnostic pop | ||
| #endif |
There was a problem hiding this comment.
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.
| 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") |
There was a problem hiding this comment.
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 {
| ^
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.