Hi,
I do wish the build was warning-free. And absolutely the warnings should not happen on headers when included by users.
I can comment a few things to clarify/guide whoever wants to clean it up:
The warnings about HlmsPassPso:
int memcmp(const void*, const void*, size_t)” specified size 18446744073709551592 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
Are right. This is a bug.
Fixed.
--------------------------------------------------------------------------------------------------
The majority of the -Wclass-memaccess warnings (all of them?) are false positives. The memsets/memcpys are intentional. GCC assumes because there's a constructor, that the object is not POD. But the constructor just initializes all values to 0 or makes a raw copy on purpose.
In particular the warnings about memcpy in OgreFastArray.h in growToFit are intentional. When we need to migrate the internal buffer, we perform a raw copy. This could indeed break very edge cases (where the iterators hold a pointer to its own internal address; which would be updated via the copy operator during iterator invalidation), but FastArray does not care about these extreme edge cases. If for some reasons this needed, then use std::vector instead.
GCC suggests to use value-initialization/copy-assignment instead of memset/memcpy. However we exactly want to avoid this, because value-initialization/copy-assignment needs to deal with hard edge cases that simply won't happen in our code and end up producing suboptimal code (or requires LTO, if it works, to be on which increases linking time a lot)
--------------------------------------------------------------------------------------------------
The fallthrough warnings for switch statements (-Wimplicit-fallthrough) are there because the code long predates the existence of this warning flag. The warnings can be silenced via a comment:
Code: Select all
switch( value )
{
case 0:
//fallthrough
case 1:
code();
break;
}
Or with C++17 "[[fallthrough]]" (we use C++03 in Ogre 2.1)
Most of these fallthrough are intentional, but I wouldn't rule out an accidental error. It's just a lot of switch() statements to analyze...
--------------------------------------------------------------------------------------------------
The -Woverflow bugs need a good look. Looks like code manages to work more by chance than anything else.
For example "size_t shadowMapIdx = 0;" in OgreScriptTranslator.cpp may be as well an uint32 to silence the warning:
Code: Select all
OgreMain/src/OgreScriptTranslator.cpp:7950:99: varoitus: conversion from ”long unsigned int” to ”unsigned int” changes value from ”18446744073709551615” to ”4294967295” [-Woverflow]
std::numeric_limits<size_t>::max() );
as parseUnsignedInt is not a size_t.
However it's not that easy because in another path we perform:
Code: Select all
shadowMapIdx = any_cast<size_t>( obj->parent->context );
which means our caller is sending us a size_t object, and thus changing shadowMapIdx type means the caller's code needs to be adjusted as well.
But everything sorts out ok because in the end we perform:
Code: Select all
(*itor)->mShadowMapIdx = static_cast<uint32>(shadowMapIdx);
TL;DR It's a tangled mess and a miracle it works at all.
--------------------------------------------------------------------------------------------------
The header from OgreMain/include/Math/Array/SSE2/Single/sse_mathfun.h came from a third party and I never bothered to fix the warnings. It "just works".
--------------------------------------------------------------------------------------------------
I do not know why GCC reports this one:
Code: Select all
RenderSystems/GL3Plus/src/OgreGL3PlusRenderSystem.cpp:1794:34: varoitus: ”minFilter” may be used uninitialized in this function [-Wmaybe-uninitialized]
OCGE( glSamplerParameteri( samplerName, GL_TEXTURE_MIN_FILTER, minFilter ) );
My best guess is that perhaps it is catching the case where either newBlock->mMinFilter or newBlock->mMipFilter contain invalid integer values (i.e. a value that is not in FilterOptions). But if that's the reason then we don't care (the problem would be WAY bigger if mMinFilter/mMipFilter is invalid)
--------------------------------------------------------------------------------------------------
The -Wignored-attributes is a tough one. For starters we don't care if they're ignored. Second, most of them come from attributes that are NOT ignored by other compilers (e.g. MSVC or Clang, depending the attribute)