* [PATCH] arm64/cache: fix -Woverride-init compiler warnings
@ 2019-08-02 15:32 Qian Cai
2019-08-05 9:52 ` Will Deacon
0 siblings, 1 reply; 6+ messages in thread
From: Qian Cai @ 2019-08-02 15:32 UTC (permalink / raw)
To: will, catalin.marinas
Cc: mark.rutland, linux-arm-kernel, linux-kernel, Qian Cai
The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged
VIVT I-caches") introduced some compiation warnings from GCC,
arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field
overwritten [-Woverride-init]
[ICACHE_POLICY_VIPT] = "VIPT",
^~~~~~
arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for
'icache_policy_str[2]')
arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field
overwritten [-Woverride-init]
[ICACHE_POLICY_PIPT] = "PIPT",
^~~~~~
arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for
'icache_policy_str[3]')
arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field
overwritten [-Woverride-init]
[ICACHE_POLICY_VPIPT] = "VPIPT",
^~~~~~~
arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for
'icache_policy_str[0]')
because it initializes icache_policy_str[0 ... 3] twice.
Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches")
Signed-off-by: Qian Cai <cai@lca.pw>
---
arch/arm64/kernel/cpuinfo.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index 876055e37352..193b38da8d96 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -34,10 +34,10 @@
static struct cpuinfo_arm64 boot_cpu_data;
static char *icache_policy_str[] = {
- [0 ... ICACHE_POLICY_PIPT] = "RESERVED/UNKNOWN",
+ [ICACHE_POLICY_VPIPT] = "VPIPT",
+ [ICACHE_POLICY_VPIPT + 1] = "RESERVED/UNKNOWN",
[ICACHE_POLICY_VIPT] = "VIPT",
[ICACHE_POLICY_PIPT] = "PIPT",
- [ICACHE_POLICY_VPIPT] = "VPIPT",
};
unsigned long __icache_flags;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64/cache: fix -Woverride-init compiler warnings
2019-08-02 15:32 [PATCH] arm64/cache: fix -Woverride-init compiler warnings Qian Cai
@ 2019-08-05 9:52 ` Will Deacon
2019-08-05 11:47 ` Qian Cai
0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2019-08-05 9:52 UTC (permalink / raw)
To: Qian Cai; +Cc: catalin.marinas, mark.rutland, linux-arm-kernel, linux-kernel
On Fri, Aug 02, 2019 at 11:32:24AM -0400, Qian Cai wrote:
> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged
> VIVT I-caches") introduced some compiation warnings from GCC,
>
> arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field
> overwritten [-Woverride-init]
> [ICACHE_POLICY_VIPT] = "VIPT",
> ^~~~~~
> arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for
> 'icache_policy_str[2]')
> arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field
> overwritten [-Woverride-init]
> [ICACHE_POLICY_PIPT] = "PIPT",
> ^~~~~~
> arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for
> 'icache_policy_str[3]')
> arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field
> overwritten [-Woverride-init]
> [ICACHE_POLICY_VPIPT] = "VPIPT",
> ^~~~~~~
> arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for
> 'icache_policy_str[0]')
>
> because it initializes icache_policy_str[0 ... 3] twice.
>
> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches")
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
> arch/arm64/kernel/cpuinfo.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index 876055e37352..193b38da8d96 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -34,10 +34,10 @@
> static struct cpuinfo_arm64 boot_cpu_data;
>
> static char *icache_policy_str[] = {
> - [0 ... ICACHE_POLICY_PIPT] = "RESERVED/UNKNOWN",
> + [ICACHE_POLICY_VPIPT] = "VPIPT",
> + [ICACHE_POLICY_VPIPT + 1] = "RESERVED/UNKNOWN",
> [ICACHE_POLICY_VIPT] = "VIPT",
> [ICACHE_POLICY_PIPT] = "PIPT",
> - [ICACHE_POLICY_VPIPT] = "VPIPT",
I really don't like this patch. Using "[0 ... MAXIDX] = <default>" is a
useful idiom and I think the code is more error-prone the way you have
restructured it.
Why are you passing -Woverride-init to the compiler anyway? There's only
one Makefile that references that option, and it's specific to a pinctrl
driver.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64/cache: fix -Woverride-init compiler warnings
2019-08-05 9:52 ` Will Deacon
@ 2019-08-05 11:47 ` Qian Cai
2019-08-05 14:01 ` Will Deacon
0 siblings, 1 reply; 6+ messages in thread
From: Qian Cai @ 2019-08-05 11:47 UTC (permalink / raw)
To: Will Deacon; +Cc: Catalin Marinas, Mark Rutland, linux-arm-kernel, linux-kernel
> On Aug 5, 2019, at 5:52 AM, Will Deacon <will@kernel.org> wrote:
>
> On Fri, Aug 02, 2019 at 11:32:24AM -0400, Qian Cai wrote:
>> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged
>> VIVT I-caches") introduced some compiation warnings from GCC,
>>
>> arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field
>> overwritten [-Woverride-init]
>> [ICACHE_POLICY_VIPT] = "VIPT",
>> ^~~~~~
>> arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for
>> 'icache_policy_str[2]')
>> arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field
>> overwritten [-Woverride-init]
>> [ICACHE_POLICY_PIPT] = "PIPT",
>> ^~~~~~
>> arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for
>> 'icache_policy_str[3]')
>> arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field
>> overwritten [-Woverride-init]
>> [ICACHE_POLICY_VPIPT] = "VPIPT",
>> ^~~~~~~
>> arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for
>> 'icache_policy_str[0]')
>>
>> because it initializes icache_policy_str[0 ... 3] twice.
>>
>> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches")
>> Signed-off-by: Qian Cai <cai@lca.pw>
>> ---
>> arch/arm64/kernel/cpuinfo.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
>> index 876055e37352..193b38da8d96 100644
>> --- a/arch/arm64/kernel/cpuinfo.c
>> +++ b/arch/arm64/kernel/cpuinfo.c
>> @@ -34,10 +34,10 @@
>> static struct cpuinfo_arm64 boot_cpu_data;
>>
>> static char *icache_policy_str[] = {
>> - [0 ... ICACHE_POLICY_PIPT] = "RESERVED/UNKNOWN",
>> + [ICACHE_POLICY_VPIPT] = "VPIPT",
>> + [ICACHE_POLICY_VPIPT + 1] = "RESERVED/UNKNOWN",
>> [ICACHE_POLICY_VIPT] = "VIPT",
>> [ICACHE_POLICY_PIPT] = "PIPT",
>> - [ICACHE_POLICY_VPIPT] = "VPIPT",
>
> I really don't like this patch. Using "[0 ... MAXIDX] = <default>" is a
> useful idiom and I think the code is more error-prone the way you have
> restructured it.
>
> Why are you passing -Woverride-init to the compiler anyway? There's only
> one Makefile that references that option, and it's specific to a pinctrl
> driver.
Those extra warnings can be enabled by “make W=1”. “-Woverride-init “ seems to be useful
to catch potential developer mistakes with unintented double-initializations. It is normal to
start to fix the most of false-positives first before globally enabling the flag by default just like
“-Wimplicit-fallthrough” mentioned in,
https://lwn.net/Articles/794944/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64/cache: fix -Woverride-init compiler warnings
2019-08-05 11:47 ` Qian Cai
@ 2019-08-05 14:01 ` Will Deacon
2019-08-06 3:50 ` Qian Cai
0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2019-08-05 14:01 UTC (permalink / raw)
To: Qian Cai; +Cc: Catalin Marinas, Mark Rutland, linux-arm-kernel, linux-kernel
On Mon, Aug 05, 2019 at 07:47:37AM -0400, Qian Cai wrote:
>
>
> > On Aug 5, 2019, at 5:52 AM, Will Deacon <will@kernel.org> wrote:
> >
> > On Fri, Aug 02, 2019 at 11:32:24AM -0400, Qian Cai wrote:
> >> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged
> >> VIVT I-caches") introduced some compiation warnings from GCC,
> >>
> >> arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field
> >> overwritten [-Woverride-init]
> >> [ICACHE_POLICY_VIPT] = "VIPT",
> >> ^~~~~~
> >> arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for
> >> 'icache_policy_str[2]')
> >> arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field
> >> overwritten [-Woverride-init]
> >> [ICACHE_POLICY_PIPT] = "PIPT",
> >> ^~~~~~
> >> arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for
> >> 'icache_policy_str[3]')
> >> arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field
> >> overwritten [-Woverride-init]
> >> [ICACHE_POLICY_VPIPT] = "VPIPT",
> >> ^~~~~~~
> >> arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for
> >> 'icache_policy_str[0]')
> >>
> >> because it initializes icache_policy_str[0 ... 3] twice.
> >>
> >> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches")
> >> Signed-off-by: Qian Cai <cai@lca.pw>
> >> ---
> >> arch/arm64/kernel/cpuinfo.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> >> index 876055e37352..193b38da8d96 100644
> >> --- a/arch/arm64/kernel/cpuinfo.c
> >> +++ b/arch/arm64/kernel/cpuinfo.c
> >> @@ -34,10 +34,10 @@
> >> static struct cpuinfo_arm64 boot_cpu_data;
> >>
> >> static char *icache_policy_str[] = {
> >> - [0 ... ICACHE_POLICY_PIPT] = "RESERVED/UNKNOWN",
> >> + [ICACHE_POLICY_VPIPT] = "VPIPT",
> >> + [ICACHE_POLICY_VPIPT + 1] = "RESERVED/UNKNOWN",
> >> [ICACHE_POLICY_VIPT] = "VIPT",
> >> [ICACHE_POLICY_PIPT] = "PIPT",
> >> - [ICACHE_POLICY_VPIPT] = "VPIPT",
> >
> > I really don't like this patch. Using "[0 ... MAXIDX] = <default>" is a
> > useful idiom and I think the code is more error-prone the way you have
> > restructured it.
> >
> > Why are you passing -Woverride-init to the compiler anyway? There's only
> > one Makefile that references that option, and it's specific to a pinctrl
> > driver.
>
> Those extra warnings can be enabled by “make W=1”. “-Woverride-init “ seems to be useful
> to catch potential developer mistakes with unintented double-initializations. It is normal to
> start to fix the most of false-positives first before globally enabling the flag by default just like
> “-Wimplicit-fallthrough” mentioned in,
>
> https://lwn.net/Articles/794944/
I think this case is completely different to the implicit fallthrough stuff.
The solution there was simply to add a comment without restructuring the
surrounding code. What your patch does here is actively make the code harder
to understand.
Initialising a static array with a non-zero pattern is a useful idiom and I
don't think we should throw that away just to appease a silly compiler
warning that appears only with non-default build options. Have a look at
the way we use PERF_MAP_ALL_UNSUPPORTED in the Arm PMU code, for example.
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64/cache: fix -Woverride-init compiler warnings
2019-08-05 14:01 ` Will Deacon
@ 2019-08-06 3:50 ` Qian Cai
2019-08-06 12:07 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Qian Cai @ 2019-08-06 3:50 UTC (permalink / raw)
To: Will Deacon; +Cc: Catalin Marinas, Mark Rutland, linux-arm-kernel, linux-kernel
> On Aug 5, 2019, at 10:01 AM, Will Deacon <will@kernel.org> wrote:
>
> On Mon, Aug 05, 2019 at 07:47:37AM -0400, Qian Cai wrote:
>>
>>
>>> On Aug 5, 2019, at 5:52 AM, Will Deacon <will@kernel.org> wrote:
>>>
>>> On Fri, Aug 02, 2019 at 11:32:24AM -0400, Qian Cai wrote:
>>>> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged
>>>> VIVT I-caches") introduced some compiation warnings from GCC,
>>>>
>>>> arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field
>>>> overwritten [-Woverride-init]
>>>> [ICACHE_POLICY_VIPT] = "VIPT",
>>>> ^~~~~~
>>>> arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for
>>>> 'icache_policy_str[2]')
>>>> arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field
>>>> overwritten [-Woverride-init]
>>>> [ICACHE_POLICY_PIPT] = "PIPT",
>>>> ^~~~~~
>>>> arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for
>>>> 'icache_policy_str[3]')
>>>> arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field
>>>> overwritten [-Woverride-init]
>>>> [ICACHE_POLICY_VPIPT] = "VPIPT",
>>>> ^~~~~~~
>>>> arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for
>>>> 'icache_policy_str[0]')
>>>>
>>>> because it initializes icache_policy_str[0 ... 3] twice.
>>>>
>>>> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches")
>>>> Signed-off-by: Qian Cai <cai@lca.pw>
>>>> ---
>>>> arch/arm64/kernel/cpuinfo.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
>>>> index 876055e37352..193b38da8d96 100644
>>>> --- a/arch/arm64/kernel/cpuinfo.c
>>>> +++ b/arch/arm64/kernel/cpuinfo.c
>>>> @@ -34,10 +34,10 @@
>>>> static struct cpuinfo_arm64 boot_cpu_data;
>>>>
>>>> static char *icache_policy_str[] = {
>>>> - [0 ... ICACHE_POLICY_PIPT] = "RESERVED/UNKNOWN",
>>>> + [ICACHE_POLICY_VPIPT] = "VPIPT",
>>>> + [ICACHE_POLICY_VPIPT + 1] = "RESERVED/UNKNOWN",
>>>> [ICACHE_POLICY_VIPT] = "VIPT",
>>>> [ICACHE_POLICY_PIPT] = "PIPT",
>>>> - [ICACHE_POLICY_VPIPT] = "VPIPT",
>>>
>>> I really don't like this patch. Using "[0 ... MAXIDX] = <default>" is a
>>> useful idiom and I think the code is more error-prone the way you have
>>> restructured it.
>>>
>>> Why are you passing -Woverride-init to the compiler anyway? There's only
>>> one Makefile that references that option, and it's specific to a pinctrl
>>> driver.
>>
>> Those extra warnings can be enabled by “make W=1”. “-Woverride-init “ seems to be useful
>> to catch potential developer mistakes with unintented double-initializations. It is normal to
>> start to fix the most of false-positives first before globally enabling the flag by default just like
>> “-Wimplicit-fallthrough” mentioned in,
>>
>> https://lwn.net/Articles/794944/
>
> I think this case is completely different to the implicit fallthrough stuff.
> The solution there was simply to add a comment without restructuring the
> surrounding code. What your patch does here is actively make the code harder
> to understand.
>
> Initialising a static array with a non-zero pattern is a useful idiom and I
> don't think we should throw that away just to appease a silly compiler
> warning that appears only with non-default build options. Have a look at
> the way we use PERF_MAP_ALL_UNSUPPORTED in the Arm PMU code, for example.
Well, both GCC and Clang would generate warnings for those. Clang even enable this by
default,
https://releases.llvm.org/8.0.0/tools/clang/docs/DiagnosticsReference.html#winitializer-overrides
Assume compiler people are sane, I probably not call those are “silly”.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arm64/cache: fix -Woverride-init compiler warnings
2019-08-06 3:50 ` Qian Cai
@ 2019-08-06 12:07 ` Mark Rutland
0 siblings, 0 replies; 6+ messages in thread
From: Mark Rutland @ 2019-08-06 12:07 UTC (permalink / raw)
To: Qian Cai; +Cc: Will Deacon, Catalin Marinas, linux-arm-kernel, linux-kernel
On Mon, Aug 05, 2019 at 11:50:03PM -0400, Qian Cai wrote:
>
>
> > On Aug 5, 2019, at 10:01 AM, Will Deacon <will@kernel.org> wrote:
> >
> > On Mon, Aug 05, 2019 at 07:47:37AM -0400, Qian Cai wrote:
> >>
> >>
> >>> On Aug 5, 2019, at 5:52 AM, Will Deacon <will@kernel.org> wrote:
> >>>
> >>> On Fri, Aug 02, 2019 at 11:32:24AM -0400, Qian Cai wrote:
> >>>> The commit 155433cb365e ("arm64: cache: Remove support for ASID-tagged
> >>>> VIVT I-caches") introduced some compiation warnings from GCC,
> >>>>
> >>>> arch/arm64/kernel/cpuinfo.c:38:26: warning: initialized field
> >>>> overwritten [-Woverride-init]
> >>>> [ICACHE_POLICY_VIPT] = "VIPT",
> >>>> ^~~~~~
> >>>> arch/arm64/kernel/cpuinfo.c:38:26: note: (near initialization for
> >>>> 'icache_policy_str[2]')
> >>>> arch/arm64/kernel/cpuinfo.c:39:26: warning: initialized field
> >>>> overwritten [-Woverride-init]
> >>>> [ICACHE_POLICY_PIPT] = "PIPT",
> >>>> ^~~~~~
> >>>> arch/arm64/kernel/cpuinfo.c:39:26: note: (near initialization for
> >>>> 'icache_policy_str[3]')
> >>>> arch/arm64/kernel/cpuinfo.c:40:27: warning: initialized field
> >>>> overwritten [-Woverride-init]
> >>>> [ICACHE_POLICY_VPIPT] = "VPIPT",
> >>>> ^~~~~~~
> >>>> arch/arm64/kernel/cpuinfo.c:40:27: note: (near initialization for
> >>>> 'icache_policy_str[0]')
> >>>>
> >>>> because it initializes icache_policy_str[0 ... 3] twice.
> >>>>
> >>>> Fixes: 155433cb365e ("arm64: cache: Remove support for ASID-tagged VIVT I-caches")
> >>>> Signed-off-by: Qian Cai <cai@lca.pw>
> >>>> ---
> >>>> arch/arm64/kernel/cpuinfo.c | 4 ++--
> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> >>>> index 876055e37352..193b38da8d96 100644
> >>>> --- a/arch/arm64/kernel/cpuinfo.c
> >>>> +++ b/arch/arm64/kernel/cpuinfo.c
> >>>> @@ -34,10 +34,10 @@
> >>>> static struct cpuinfo_arm64 boot_cpu_data;
> >>>>
> >>>> static char *icache_policy_str[] = {
> >>>> - [0 ... ICACHE_POLICY_PIPT] = "RESERVED/UNKNOWN",
> >>>> + [ICACHE_POLICY_VPIPT] = "VPIPT",
> >>>> + [ICACHE_POLICY_VPIPT + 1] = "RESERVED/UNKNOWN",
> >>>> [ICACHE_POLICY_VIPT] = "VIPT",
> >>>> [ICACHE_POLICY_PIPT] = "PIPT",
> >>>> - [ICACHE_POLICY_VPIPT] = "VPIPT",
> >>>
> >>> I really don't like this patch. Using "[0 ... MAXIDX] = <default>" is a
> >>> useful idiom and I think the code is more error-prone the way you have
> >>> restructured it.
> >>>
> >>> Why are you passing -Woverride-init to the compiler anyway? There's only
> >>> one Makefile that references that option, and it's specific to a pinctrl
> >>> driver.
> >>
> >> Those extra warnings can be enabled by “make W=1”. “-Woverride-init “ seems to be useful
> >> to catch potential developer mistakes with unintented double-initializations. It is normal to
> >> start to fix the most of false-positives first before globally enabling the flag by default just like
> >> “-Wimplicit-fallthrough” mentioned in,
> >>
> >> https://lwn.net/Articles/794944/
> >
> > I think this case is completely different to the implicit fallthrough stuff.
> > The solution there was simply to add a comment without restructuring the
> > surrounding code. What your patch does here is actively make the code harder
> > to understand.
> >
> > Initialising a static array with a non-zero pattern is a useful idiom and I
> > don't think we should throw that away just to appease a silly compiler
> > warning that appears only with non-default build options. Have a look at
> > the way we use PERF_MAP_ALL_UNSUPPORTED in the Arm PMU code, for example.
>
> Well, both GCC and Clang would generate warnings for those. Clang even enable this by
> default,
>
> https://releases.llvm.org/8.0.0/tools/clang/docs/DiagnosticsReference.html#winitializer-overrides
>
> Assume compiler people are sane, I probably not call those are “silly”.
We're not disputing the sanity of compiler folk; Will did not say
anything about that.
The warning is unhelpful in the case of the [0 ... MAXIDX] = <default>
idiom, and the modification you suggest:
* Makes the code harder to read.
* Increases the necessary context. e.g. I must know the specific values
of the enum to know that ICACHE_POLICY_VPIPT + 1 is the unallocated
slot.
* Less robust. If the enum gets re-ordered, we must update the array.
If the enum is expanded, new elements must be added to the array to
initialize entries to the default value, which also makes the code
more verbose and painful to read. IIUC if we don't explicitly
initialize an element, we won't get a warning, which would be harmful.
If there's some way to mark the default initialization as overridable,
I think that would be fine, e.g.
struct foo *array[] = {
[0 ... MAXIDX] __default = <default>,
[SOMEIDX] = <someval>,
[OTHERIDX] = <otherval>,
}
We have a number of cases where the [0 ... MAXIDX] = <default> idiom are
used, and I don't think that any of them should be changed in the manner
suggested by this patch.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-06 12:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-02 15:32 [PATCH] arm64/cache: fix -Woverride-init compiler warnings Qian Cai
2019-08-05 9:52 ` Will Deacon
2019-08-05 11:47 ` Qian Cai
2019-08-05 14:01 ` Will Deacon
2019-08-06 3:50 ` Qian Cai
2019-08-06 12:07 ` Mark Rutland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).