Silence false-positive warning for malformed placement new on GCC 11.1

[lightspot21]

This closes #52119. Right now I am moving the offset to its own variable in order to calm down GCC's warning. Better ideas always welcome.

[lightspot21]

I did quite a bit of research on why this warning pops up here.

More specifically, it's because GCC thinks that mem_new is the beginning of a new allocation, but suddenly there is a subtraction, causing it to think that we want to place the SafeNumeric outside the allocation. This seems to be the case, as GCC complains only on subtraction, not addition.

GCC doesn't know though (and cannot know, because the allocation is dynamic) that in this case, the pointer returned by Memory::alloc_static() is already offset by 16 bytes, so writing "behind" it is actually fine, as the memory needed is already allocated. (IMHO it's kinda weird though, returning a pointer in the middle of an allocation - maybe this can be refactored somehow?)

Assigning to a new variable fools the check, because the compiler thinks that this is the beginning of a separate array, and no such subtraction is done.

Therefore, the warning seems to be a false positive logic-wise, but is valid because it's not immediately clear to an outside observer that the subtraction is OK. I propose that we stay with the variables along with a comment, as they both satisfy GCC and make it clear that the subtraction is fine and done for a purpose.

Any better ideas welcome. @Calinou @godotengine/core ?

[lightspot21]

@godotengine/core ?

[lightspot21]

@nathanfranke please let me know if I changed this correctly

[nathanfranke]

@nathanfranke please let me know if I changed this correctly

Yes, looks fine. Although maybe another core contributor can look at it, since I think there are some exceptions or issues that can arise from using references.

[RandomShaper]

Considering that the reason for those interim pointers will most likely be no apparent to a future reader of that code, I'd suggest adding a comment about their purpose. However, if we take into account the size of the base patch plus comments and also that the goal is to silence a really broken warning, I'd suggest going with this:

• Around the top of the file:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wplacement-new"

• Around the bottom of the file:

#pragma GCC diagnostic pop

Anyone expected to maintain this file will know how to understand it.

[lightspot21]

@RandomShaper done

[akien-mga]

Might be worth amending the commit title as we're no longer fixing the warning but silencing it as a false positive.

[lightspot21]

Might be worth amending the commit title as we're no longer fixing the warning but silencing it as a false positive.

@akien-mga done and done

[akien-mga]

Thanks!