On Wed, Sep 12, 2018 at 4:13 PM Nick Desaulniers <ndesaulniers at google.com> wrote: > On Wed, Sep 12, 2018 at 1:24 PM Richard Smith <richardsmith at google.com> > wrote: > > > > On Wed, Sep 12, 2018 at 10:38 AM Nick Desaulniers < > ndesaulniers at google.com> wrote: > >> > >> On Tue, Sep 11, 2018 at 5:26 PM Nathan Chancellor > >> <natechancellor at gmail.com> wrote: > >> > > >> > Clang warns if there are missing braces around a subobject > >> > initializer. > >> > > >> > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c:1447:41: warning: suggest braces > >> > around initialization of subobject [-Wmissing-braces] > >> > struct amdgpu_task_info task_info = { 0 }; > >> > ^ > >> > {} > >> > 1 warning generated. > >> > > >> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c:262:41: warning: suggest braces > >> > around initialization of subobject [-Wmissing-braces] > >> > struct amdgpu_task_info task_info = { 0 }; > >> > ^ > >> > {} > >> > 1 warning generated. > >> > > >> > Reported-by: Nick Desaulniers <ndesaulniers at google.com> > >> > Signed-off-by: Nathan Chancellor <natechancellor at gmail.com> > >> > --- > >> > drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 2 +- > >> > drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 2 +- > >> > 2 files changed, 2 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c > b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c > >> > index 9333109b210d..968cc1b8cdff 100644 > >> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c > >> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c > >> > @@ -1444,7 +1444,7 @@ static int gmc_v8_0_process_interrupt(struct > amdgpu_device *adev, > >> > gmc_v8_0_set_fault_enable_default(adev, false); > >> > > >> > if (printk_ratelimit()) { > >> > - struct amdgpu_task_info task_info = { 0 }; > >> > + struct amdgpu_task_info task_info = { { 0 } }; > >> > >> Hi Nathan, > >> Thanks for this patch. I discussed this syntax with our language > >> lawyers. Turns out, this is not quite correct, as you're now saying > >> "initialize the first subobject to zero, but not the rest of the > >> object." -Wmissing-field-initializers would highlight this, but it's > >> not part of -Wall. It would be more correct to zero initialize the > >> full struct, including all of its subobjects with `= {};`. > > > > > > Sorry, I think I've caused some confusion here. > > > > Elements with an omitted initializer get implicitly zero-initialized. In > C++, it's idiomatic to write `= {}` to perform aggregate > zero-initialization, but in C, that's invalid because at least one > initializer is syntactically required within the braces. As a result, `= > {0}` is an idiomatic way to perform zero-initialization of an aggregate in > C. > > That doesn't seem to be the case: > https://godbolt.org/z/TZzfo6 shouldn't Clang warn in the case of bar()? > This is a GNU extension. Use -pedantic-errors to turn off extensions, then Clang and GCC reject bar(): https://godbolt.org/z/pIzI6M > > Clang intends to suppress the -Wmissing-braces in that case; if the > warning is still being produced in a recent version of Clang, that's a bug. > However, the warning suppression was added between Clang 5 and Clang 6, so > it's very plausible that the compiler being used here is simply older than > the warning fix. > > > > (Long story short: the change here seems fine, but should be unnecessary > as of Clang 6.) > > The warning was identified from clang-8 ToT synced yesterday. > Thanks for the testcase. This is a Clang bug. Apparently the warning suppression only works when the first member is of type 'int'! https://godbolt.org/z/sxnZvv -------------- next part -------------- An HTML attachment was scrubbed... URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20180913/bd8a6cc1/attachment.html>