| Message ID | 7cee38788a7a3c2c09a238e01c5bd825445f999d.1665085395.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix syntax errors under clang 11.0.0 on MacOS | expand |
Am 06.10.22 um 21:43 schrieb Jeff Hostetler via GitGitGadget: > From: Jeff Hostetler <jeffhost@microsoft.com> > > Add extra set of braces around zero initialization of two array/structure > variables to resolve compiler errors/warnings from clang 11.0.0 on MacOS. > This is not needed on clang 14.0. Not sure how the Apple version numbers map to LLVM versions. I can reproduce it with Godbolt's Compiler Explorer with clang 8, but not with clang 9 or higher: https://godbolt.org/z/f7f7s9xxz Funny that type of the "inner" member seems to affect the warning. > > $ uname -a > Darwin jeffhost-mbp.local 19.6.0 Darwin Kernel Version 19.6.0: \ > Mon Apr 18 21:50:40 PDT 2022; \ > root:xnu-6153.141.62~1/RELEASE_X86_64 x86_64 > $ clang -v > Apple clang version 11.0.0 (clang-1100.0.33.17) > Target: x86_64-apple-darwin19.6.0 > [...] > > $ make builtin/merge-file.o > CC builtin/merge-file.o > builtin/merge-file.c:29:23: error: suggest braces around initialization \ > of subobject [-Werror,-Wmissing-braces] > mmfile_t mmfs[3] = { 0 }; > ^ > {} {0} is an idiom to zero-initialize any struct, no matter how deeply nested. It's valid C and the compiler warning about it is not helpful. Shouldn't we rather silence it with -Wno-missing-braces? On the other hand: Uglifying just three initializations in total isn't that bad. The issue may reappear, though, because most people use compilers that don't issue that spurious warning, I imagine. > builtin/merge-file.c:31:20: error: suggest braces around initialization \ > of subobject [-Werror,-Wmissing-braces] > xmparam_t xmp = { 0 }; > ^ > {} > 2 errors generated. > make: *** [builtin/merge-file.o] Error 1 > > Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com> > --- > builtin/merge-file.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/builtin/merge-file.c b/builtin/merge-file.c > index c923bbf2abb..607c3d3f9e1 100644 > --- a/builtin/merge-file.c > +++ b/builtin/merge-file.c > @@ -26,9 +26,9 @@ static int label_cb(const struct option *opt, const char *arg, int unset) > int cmd_merge_file(int argc, const char **argv, const char *prefix) > { > const char *names[3] = { 0 }; > - mmfile_t mmfs[3] = { 0 }; > + mmfile_t mmfs[3] = { { 0 } }; > mmbuffer_t result = { 0 }; > - xmparam_t xmp = { 0 }; > + xmparam_t xmp = { { 0 } }; > int ret = 0, i = 0, to_stdout = 0; > int quiet = 0; > struct option options[] = {
On Thu, Oct 6, 2022 at 5:25 PM René Scharfe <l.s.r@web.de> wrote: > Am 06.10.22 um 21:43 schrieb Jeff Hostetler via GitGitGadget: > > Add extra set of braces around zero initialization of two array/structure > > variables to resolve compiler errors/warnings from clang 11.0.0 on MacOS. > > This is not needed on clang 14.0. > > Not sure how the Apple version numbers map to LLVM versions. I can > reproduce it with Godbolt's Compiler Explorer with clang 8, but not > with clang 9 or higher: https://godbolt.org/z/f7f7s9xxz > > > builtin/merge-file.c:29:23: error: suggest braces around initialization \ > > of subobject [-Werror,-Wmissing-braces] > > mmfile_t mmfs[3] = { 0 }; > > ^ > > {} > > {0} is an idiom to zero-initialize any struct, no matter how deeply > nested. It's valid C and the compiler warning about it is not helpful. > Shouldn't we rather silence it with -Wno-missing-braces? That was indeed Ævar's suggestion in [1]: Since this is only a warning, and only a practical issue with -Werror I wonder if a config.mak.dev change wouldn't be better, i.e. to provide a -Wno-missing-braces for this older clang version. The problem is that Apple seems to invent their version numbers out of thin air with no relation to reality[2,3], so it may take some effort to work out suitable "version mapping rules" for config.mak.dev, and nobody has done it yet. [1]: https://lore.kernel.org/git/220712.86lesy6cri.gmgdl@evledraar.gmail.com/ [2]: https://lore.kernel.org/git/Ys0hhYjPExuNWynE@coredump.intra.peff.net/ [3]: https://lore.kernel.org/git/YQ2LdvwEnZN9LUQn@coredump.intra.peff.net/
Eric Sunshine <sunshine@sunshineco.com> writes: >> Shouldn't we rather silence it with -Wno-missing-braces? > > That was indeed Ævar's suggestion in [1]: > > Since this is only a warning, and only a practical issue with > -Werror I wonder if a config.mak.dev change wouldn't be better, > i.e. to provide a -Wno-missing-braces for this older clang > version. > > The problem is that Apple seems to invent their version numbers out of > thin air with no relation to reality[2,3], so it may take some effort > to work out suitable "version mapping rules" for config.mak.dev, and > nobody has done it yet. > > [1]: https://lore.kernel.org/git/220712.86lesy6cri.gmgdl@evledraar.gmail.com/ > [2]: https://lore.kernel.org/git/Ys0hhYjPExuNWynE@coredump.intra.peff.net/ > [3]: https://lore.kernel.org/git/YQ2LdvwEnZN9LUQn@coredump.intra.peff.net/ I agree that telling affected users to squelch the warning is a good first step, and automating it, if there is an easy implementation to do so, would be a good thing to have on top. Thanks for digging up the thread.
diff --git a/builtin/merge-file.c b/builtin/merge-file.c index c923bbf2abb..607c3d3f9e1 100644 --- a/builtin/merge-file.c +++ b/builtin/merge-file.c @@ -26,9 +26,9 @@ static int label_cb(const struct option *opt, const char *arg, int unset) int cmd_merge_file(int argc, const char **argv, const char *prefix) { const char *names[3] = { 0 }; - mmfile_t mmfs[3] = { 0 }; + mmfile_t mmfs[3] = { { 0 } }; mmbuffer_t result = { 0 }; - xmparam_t xmp = { 0 }; + xmparam_t xmp = { { 0 } }; int ret = 0, i = 0, to_stdout = 0; int quiet = 0; struct option options[] = {