| Message ID | 20190515043753.9853-1-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: drop unneeded -Wall addition | expand |
On Wed, May 15, 2019 at 3:25 PM Chris Wilson <chris@chris-wilson.co.uk> wrote: > > Quoting Masahiro Yamada (2019-05-15 05:37:53) > > The top level Makefile adds -Wall globally: > > > > KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \ > > > > I see two "-Wall" added for compiling under drivers/gpu/drm/i915/. > > Does it matter? Is the statement in i915/Makefile not more complete for > saying "-Wall -Wextra -Werror"? Not fatal, but better to fix. Why not fix the comment if you mind "-Wall" in the comment? It will be easy to rephrase the comments without explicitly mentioning -Wall or -Wextra. I reworded it more concisely: # We aggressively eliminate warnings, # so here are more warning options than default. That's it. The CI is your local matter. Distracting comments should not be added in the upstream code in the first place. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > --- > > > > BTW, I have a question in the comment: > > > > "Note the danger in using -Wall -Wextra is that when CI updates gcc we > > will most likely get a sudden build breakage... Hopefully we will fix > > new warnings before CI updates!" > > > > Enabling whatever warning options does not cause build breakage. > > -Werror does. > > > > So, I think the correct statement is: > > > > "Note the danger in using -Werror is that when CI updates gcc we ... > > No. Heh, I thought the answer was Yes, since I saw the following in this Makefile. # Add a set of useful warning flags and enable -Werror for CI to prevent > CI enforces -Werror and that is constant, so the uncontrolled > variable, the danger, lies in using the unreliable heuristics gcc may > arbitrary enable between versions. That the set of warnings causing an > error may be different between CI and the developer. > -Chris
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index fbcb0904f4a8..4a4f60c7edfc 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -12,7 +12,7 @@ # Note the danger in using -Wall -Wextra is that when CI updates gcc we # will most likely get a sudden build breakage... Hopefully we will fix # new warnings before CI updates! -subdir-ccflags-y := -Wall -Wextra +subdir-ccflags-y := -Wextra subdir-ccflags-y += $(call cc-disable-warning, unused-parameter) subdir-ccflags-y += $(call cc-disable-warning, type-limits) subdir-ccflags-y += $(call cc-disable-warning, missing-field-initializers)
The top level Makefile adds -Wall globally: KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \ I see two "-Wall" added for compiling under drivers/gpu/drm/i915/. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- BTW, I have a question in the comment: "Note the danger in using -Wall -Wextra is that when CI updates gcc we will most likely get a sudden build breakage... Hopefully we will fix new warnings before CI updates!" Enabling whatever warning options does not cause build breakage. -Werror does. So, I think the correct statement is: "Note the danger in using -Werror is that when CI updates gcc we ... ^^^^^^^ drivers/gpu/drm/i915/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)