Peter Eisentraut <[email protected]> writes:
> It looks like all of the changes are of the form
> - ErrorSaveContext escontext = {T_ErrorSaveContext};
> + ErrorSaveContext escontext = {.type = T_ErrorSaveContext};
Right, that was my understanding too.
> Maybe we could do this more elegantly and safer with a non-pointer
> variant of makeNode/newNode.
> Like
> #define initNode(_type_) ((_type_){.type = T_##_type_})
> ErrorSaveContext escontext = initNode(ErrorSaveContext);
Hmm, that would reduce the risk of writing the wrong T_Foo constant in
this context, but it doesn't seem to me that that risk is high as-is.
Macro wrapper or not, I'm still unclear on why we should think that
this change improves anything. It seems 100.00% arbitrary that this
particular compiler switch produces a warning about the first usage
and not the second, or that some other compiler wouldn't produce a
warning about the second usage. I do see that the gcc manual
documents -Wmissing-field-initializers as behaving this way, but
that doesn't make it any less random.
So my answer to this is "don't use -Wmissing-field-initializers".
The only argument I can see in favor of trying to make our build
clean with that is if we were going to try to be clean with
-Wextra in general. But a quick check says that -Wextra still
produces thousands of lines of pedantry.
In any case, I'd want to see an explicit project decision to make
cleanliness against any new warning switch be a supported thing.
That would probably mean turning it on by default, not just
occasionally accepting drive-by patches about it.
regards, tom lane
