Re: [PATCH v3] drm/i915: Add -Wall -Wextra to our build, set warnings to full

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On Mon, 2017-10-16 at 14:09 +0100, Chris Wilson wrote:
> Recently W=1 on gcc-7.2 (-Wunused-const-variable) caught a regression
> that had been lurking for 6 months, so lets try enabling the full set of
> warnings for CI builds. This means more patches will be rejected early
> that contain trivial and sometimes not so trivial bugs. However, our
> code does not yet compile cleanly with W=1, so we have to apply a filter
> to the set of warnings until we can eliminate the mistakes. It also
> means that developers will have to be running the full gamut of gcc to
> ensure that as warnings come and go with gcc updates, we have the CI
> build prepared.
> 
> v2: Use fine-grained -Wno overrides. Inside the makefile, we can
> specify CFLAGS on a per-object level, which allows us to limit the scope
> of any particular warning override.
> v3: Place per-file overrides after the main enabling block.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Acked-by: Tomi Sarvela <tomi.p.sarvela@intel.com>

<SNIP>

> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -2,7 +2,26 @@
>  # Makefile for the drm device driver.  This driver provides support for the
>  # Direct Rendering Infrastructure (DRI) in XFree86 4.1.0 and higher.
>  
> -subdir-ccflags-$(CONFIG_DRM_I915_WERROR) := -Werror
> +# Add a set of useful warning flags and enable -Werror for CI to prevent
> +# trivial mistakes from creeping in. We have to do this piecemeal as we reject
> +# any patch that isn't warning clean, so turning on -Wall -Wextra (or W=1) we
> +# need to filter out dubious warnings.  Still it is our interest
> +# to keep running locally with W=1 C=1 until we are completely clean.
> +#
> +# 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 += $(call cc-option,-Wno-unused-parameter,)
> +subdir-ccflags-y += $(call cc-option,-Wno-type-limits,)
> +subdir-ccflags-y += $(call cc-option,-Wno-missing-field-initializers,)
> +subdir-ccflags-y += $(call cc-option,-Wno-implicit-fallthrough,)
> +subdir-ccflags-$(CONFIG_DRM_I915_WERROR) += -Werror
> +
> +# Fine grained warnings disable
> +CFLAGS_i915_pci.o = $(call cc-option,-Wno-override-init,)
> +CFLAGS_intel_fbdev.o = $(call cc-option,-Wno-override-init,)

Then there's the option of tying these tightly to next line after the
respective file is added to obj-y, but I think I prefer this most.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

But lets wait for more comments before proceeding with merge.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx