[PATCH] drm/i915: drop unneeded -Wall addition

[PATCH] drm/i915: drop unneeded -Wall addition

[Masahiro Yamada]

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(-)

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)
-- 
2.17.1

Re: [PATCH] drm/i915: drop unneeded -Wall addition

[Chris Wilson]

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"?

> 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. 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

Re: [PATCH] drm/i915: drop unneeded -Wall addition

[Masahiro Yamada]

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



-- 
Best Regards
Masahiro Yamada