[Mesa-dev] [PATCH 1/8] nir: Silence missing field initializer warnings for nir_src

[Jason Ekstrand]

On Mon, Dec 14, 2015 at 5:12 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 12/14/2015 04:39 PM, Ilia Mirkin wrote:
>> On Mon, Dec 14, 2015 at 7:28 PM, Ian Romanick <idr at freedesktop.org> wrote:
>>> On 12/14/2015 03:38 PM, Ilia Mirkin wrote:
>>>> It's a pretty standard feature of compilers to init things to 0 and
>>>> not have the full structure specified like that... what compiler are
>>>> you seeing these with? Can we just fix the glitch with a
>>>> -Wno-stupid-warnings?
>>>
>>> I have observed this with several versions of GCC.
>>>
>>> In C, you can avoid this with a trailing comma like:
>>>
>>> #define NIR_SRC_INIT (nir_src) { { NULL }, }
>>>
>>> However, nir.h is also used in some C++ code where that doesn't help.
>>>
>>> To be honest, I'm not a big fan of these macros.  Without C99 designated
>>> initalizers, maintaining initializers like these (or the ones in
>>> src/glsl/builtin_variables.cpp) is a real pain.  We can't use those, and
>>> we can't use C++ constructors.  We have no good options available. :(
>>>
>>> I thought about replacing them with a static inline function that
>>> returns a zero-initialized struct.  The compiler should generate the
>>> same code.  However, that doesn't work with uses like those in patch 3.
>>>
>>> I'm also a little curious why you didn't raise this issue when I sent
>>> these patches out in August.  I removed the patch from the series that
>>> you objected to back then.
>>
>> I have absolutely no recollection of any of that. Perhaps I saw "nir"
>> and thought to myself, "don't care, let them do whatever, this won't
>> ever affect me". Which is a sentiment I'm happy to continue with, by
>> the way.
>
> Fair enough. :)  The patch I removed was one that removed the gl_context
> parameter from a function in dd_function_table.
>
> http://patchwork.freedesktop.org/patch/58048/
>
>> I know that doing
>>
>> x = {}
>>
>> is a gcc extension, but I thought that {0} should always work (with
>> enough {} nesting in case the first element is a struct). Perhaps it
>
> {0} is, basically what we're doing now, and GCC complains about it with
> -Wmissing-field-initializers or -Wextra.  When we added C-style struct

I'm not a big fan of spending time fixing warnings that you have to
add -Wextra to get.  However, if there are C++ issues, then those
definitely need to get fixed.

> and array initializers to GLSL, we discussed adding this sort of
> implicit zero initialization.  I did some digging in the C89 and C99
> specs, and I have some recollection that in this case the missing fields
> get undefined values... but, starting with C99, {0, } implicitly
> initializes the missing fields to zero.  I also seem to recall that bit
> of weirdness in C is why quite a few people were opposed to adding it to
> GLSL.  This was several years ago, so my memory may not be completely
> reliable.
>
>> doesn't in C++? I could believe that, although I'd be surprised.
>
> The initializer support in C++ intentionally quite a bit more primitive
> than in C99.  The language designers want you to use constructors
> whether it's the best tool for the job or not... which is why there are
> no designated initializers.

So, I've got a patch somewhere that switches based on __cplusplus and
defines NIR_SRC_INIT as either the C99 thing or nir_src() for C++.
Would that solve this problem?  There was also a bug recently about us
not building with oricle studio that it would probably fix.  If so,
let's do that rather than a gigantic mess of braces and zeros.
--Jason

>> Anyways, didn't mean to stir the pot too much, just thought there
>> might be a simpler way out of all this.
>
> Well, there are. :) We just can't use them due to some combination of
> MSVC, C++, and C99.
>
>> Cheers,
>>
>>   -ilia
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev