Re: [RFC PATCH 4/4] ui/cocoa: Ignore 'initializer overrides' warnings

[Christian Schoenebeck]

On Dienstag, 15. Februar 2022 14:45:00 CET Peter Maydell wrote:
> On Tue, 15 Feb 2022 at 13:18, Christian Schoenebeck
> 
> <qemu_oss@crudebyte.com> wrote:
> > On Dienstag, 15. Februar 2022 13:06:25 CET Philippe Mathieu-Daudé via 
wrote:
> > > We globally ignore the 'initializer overrides' warnings in C code
> > > since commit c1556a812a ("configure: Disable (clang)
> > > initializer-overrides warnings"). Unfortunately the ${gcc_flags}
> > > variable is not propagated to Objective-C flags ($OBJCFLAGS).
> > > Instead of reworking the configure script to test all supported
> > > C flags in Objective-C and sanitize them, ignore the warning
> > > locally with the GCC diagnostic #pragma (Clang ignores it).
> > > 
> > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > ---
> > 
> > What about this instead:
> > 
> > diff --git a/ui/cocoa.m b/ui/cocoa.m
> > index ac18e14ce0..df9b9be999 100644
> > --- a/ui/cocoa.m
> > +++ b/ui/cocoa.m
> > @@ -652,9 +652,7 @@ QemuCocoaView *cocoaView;
> > 
> >      /* translates Macintosh keycodes to QEMU's keysym */
> > 
> > -    int without_control_translation[] = {
> > -        [0 ... 0xff] = 0,   // invalid key
> > -
> > +    int without_control_translation[256] = {
> 
> I think this won't zero-initialize, because this is a function
> level variable, not a global or static, but maybe ObjectiveC
> rules differ from C here? (Besides, it really should be
> a static const array.) That said...

That's a base requirement for designated initializers to fallback to automatic 
default initialization (zero) for omitted members. It's like:

        int a[10] = { }; // all zero

It works for me (including on function level) with both GCC and clang, on 
Linux and macOS:

#include <stdio.h>

int main() {
    int a[] = {
        [4] = 409,
        [6] = 609,
        [9] = 909
    };
    for (int i = 0; i < 10; ++i)
        printf("a[%d] = %d\n", i, a[i]);
    return 0;
}

Was this ever not the case?

> > That warning should only be raised on overlapping designated
> > initializations which strictly is undefined behaviour. Because which
> > order defines the precedence of overlapping initializers, is it the order
> > of the designated intializer list, or rather the memory order?
> 
> This is defined: textually last initializer wins.
> https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
> 
> > See also:
> > https://stackoverflow.com/questions/40920714/is-full-followed-by-partial-i
> > nitialization-of-a-subobject-undefined-behavior
> That's about struct initializers; we're dealing with array initializers
> here.

Yes, but if you suppress this warning globally, you shut up the potential 
warning for the linked case as well. And as demonstrated there, you would end 
up with different/unexpected behaviour depending on the precise compiler being 
used.

So I still think it is not a good idea to suppress this warning globally.

Best regards,
Christian Schoenebeck

> > So I have my doubts whether this warning suppression should be used in
> > QEMU at all.
> 
> The warning is useful in the pure-standard-C world where there
> is no such thing as the "range initialization" syntax. In that
> case trying to initialize the same array member twice is very
> likely a bug. However, if you use range initialization then
> the pattern "initialize a range first, then override some specific
> members within it" is very common and the warning is incorrect here.
> We use and like the range-initialization syntax, and so we disable
> the -Winitializer-overrides warnings. The bug here is just that
> we are incorrectly failing to apply the warning flags we use
> for C code when we compile ObjC files.
> 
> thanks
> -- PMM