[Mesa-dev] [Nine] 'meson: add -Werror=empty-body to disallow `if(x); `' - 'broke' Nine

Timur Kristóf timur.kristof at gmail.com
Fri Oct 25 11:09:30 UTC 2019


I think it depends on how much time it adds to the CI.
If the overhead is negligible, then it makes a lot of sense to have CI
make a release build in addition to the debug ones.

While we are at it:

Would it be possible to add some CI tests to ensure that Nine doesn't
break (even if it builds), similarly to how some drivers run their CTS
tests in there? For instance, can we run Xnine or some other small
testing tool in the CI?

What do you think about this, Axel?

Thanks & best regards,
Tim

On Fri, 2019-10-25 at 11:48 +0100, Eric Engestrom wrote:
> (for the record, the MR has been reviewed and will be merged when the
> pipeline comes back all green)
> 
> The problem is that the CI does debug builds, while this was
> a release-build-only issue.
> 
> We could perform multiple types of builds each time, but that would
> multiply the CI time/cost; I'll take a look now to see the best way
> to
> do that and see if the time cost is reasonable.
> (Meson has 6 build types, so testing all of them is not reasonable,
> but
> maybe we can test debug and release)
> 
> 
> On Friday, 2019-10-25 10:49:04 +0200, Timur Kristóf wrote:
> > Hi guys,
> > 
> > How is it possible that the CI didn't detect this problem?
> > Isn't there at least one build in the CI system which builds Nine?
> > 
> > I've created a MR to deal with it:
> > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2465
> > 
> > Best regards,
> > Tim
> > 
> > 
> > On Thu, 2019-10-24 at 23:31 +0200, Axel Davy wrote:
> > > Hi Dieter,
> > > 
> > > Maybe the best fix would be to change the definition of WARN and
> > > DBG 
> > > when DEBUG is disabled.
> > > 
> > > The definitions are in nine_debug.h
> > > 
> > > I haven't tried by maybe using "(void)" instead of nothing would
> > > work
> > > ?
> > > 
> > > Yours,
> > > 
> > > Axel
> > > 
> > > On 24/10/2019 16:34, Dieter Nützel wrote:
> > > > Hello Eric,
> > > > 
> > > > your mentioned commit
> > > > (8d43e2b2ded0fe3c82d49561cdab9f208f9e64b6)
> > > > broke 
> > > > building with NIne (-Dgallium-nine=true) for me.
> > > > 
> > > > starting with
> > > > [-]
> > > > e_st at sta/cubetexture9.c.o' -c 
> > > > ../src/gallium/state_trackers/nine/cubetexture9.c
> > > > ../src/gallium/state_trackers/nine/cubetexture9.c: In function 
> > > > ‘NineCubeTexture9_ctor’:
> > > > ../src/gallium/state_trackers/nine/cubetexture9.c:108:43:
> > > > error: 
> > > > suggest braces around empty body in an ‘if’ statement 
> > > > [-Werror=empty-body]
> > > >   108 |             "but this is unimplemented\n");
> > > >       |                                           ^
> > > > cc1: some warnings being treated as errors
> > > > 
> > > > -- 
> > > > Next
> > > > 
> > > > /surface9.c.o' -c ../src/gallium/state_trackers/nine/surface9.c
> > > > ../src/gallium/state_trackers/nine/surface9.c: In function 
> > > > ‘NineSurface9_GetContainer’:
> > > > ../src/gallium/state_trackers/nine/surface9.c:334:40: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >   334 |         DBG("QueryInterface FAILED!\n");
> > > >       |                                        ^
> > > > cc1: some warnings being treated as errors
> > > > 
> > > > -- 
> > > > 
> > > > @sta/swapchain9.c.o' -c
> > > > ../src/gallium/state_trackers/nine/swapchain9.c
> > > > ../src/gallium/state_trackers/nine/swapchain9.c: In function
> > > > ‘present’:
> > > > ../src/gallium/state_trackers/nine/swapchain9.c:737:51: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >   737 |             pSourceRect->top, pSourceRect->bottom);
> > > >       |                                                   ^
> > > > ../src/gallium/state_trackers/nine/swapchain9.c:741:47: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >   741 |             pDestRect->top, pDestRect->bottom);
> > > >       |                                               ^
> > > > cc1: some warnings being treated as errors
> > > > 
> > > > -- 
> > > > 
> > > > evice9.c.o' -c ../src/gallium/state_trackers/nine/device9.c
> > > > ../src/gallium/state_trackers/nine/device9.c: In function 
> > > > ‘NineDevice9_ctor’:
> > > > ../src/gallium/state_trackers/nine/device9.c:296:49: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >   296 |         DBG("\033[1;32mCSMT is active\033[0m\n");
> > > >       |                                                 ^
> > > > ../src/gallium/state_trackers/nine/device9.c: In function 
> > > > ‘create_zs_or_rt_surface’:
> > > > ../src/gallium/state_trackers/nine/device9.c:1221:87: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >  1221 |       DBG("FIXME Used shared handle! This option isn't 
> > > > probably handled correctly!\n");
> > > > >                      ^
> > > > ../src/gallium/state_trackers/nine/device9.c: In function 
> > > > ‘NineDevice9_UpdateSurface’:
> > > > ../src/gallium/state_trackers/nine/device9.c:1307:53: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >  1307 |             pSourceRect->right, pSourceRect->bottom);
> > > >       |                                                     ^
> > > > ../src/gallium/state_trackers/nine/device9.c:1309:68: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >  1309 |         DBG("pDestPoint = (%u,%u)\n", pDestPoint->x, 
> > > > pDestPoint->y);
> > > > >   ^
> > > > ../src/gallium/state_trackers/nine/device9.c: In function 
> > > > ‘NineDevice9_StretchRect’:
> > > > ../src/gallium/state_trackers/nine/device9.c:1588:53: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >  1588 |             pSourceRect->right, pSourceRect->bottom);
> > > >       |                                                     ^
> > > > ../src/gallium/state_trackers/nine/device9.c:1591:49: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >  1591 |             pDestRect->right, pDestRect->bottom);
> > > >       |                                                 ^
> > > > ../src/gallium/state_trackers/nine/device9.c: In function 
> > > > ‘NineDevice9_ColorFill’:
> > > > ../src/gallium/state_trackers/nine/device9.c:1786:41: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >  1786 |             pRect->right, pRect->bottom);
> > > >       |                                         ^
> > > > ../src/gallium/state_trackers/nine/device9.c: In function 
> > > > ‘NineDevice9_CreateOffscreenPlainSurface’:
> > > > ../src/gallium/state_trackers/nine/device9.c:1864:43: error:
> > > > suggest 
> > > > braces around empty body in an ‘if’ statement [-Werror=empty-
> > > > body]
> > > >  1864 |         DBG("Failed to create surface.\n");
> > > >       |                                           ^
> > > > cc1: some warnings being treated as errors
> > > > 
> > > > -- 
> > > > 
> > > > st at sta/nine_shader.c.o' -c 
> > > > ../src/gallium/state_trackers/nine/nine_shader.c
> > > > ../src/gallium/state_trackers/nine/nine_shader.c: In function 
> > > > ‘tx_dst_param_as_src’:
> > > > ../src/gallium/state_trackers/nine/nine_shader.c:1437:52:
> > > > error: 
> > > > suggest braces around empty body in an ‘if’ statement 
> > > > [-Werror=empty-body]
> > > >  1437 |         WARN("mask is 0, using identity swizzle\n");
> > > >       |
> > > > 
> > > > -- 
> > > > Last
> > > > 
> > > > ine_ff.c.o' -c ../src/gallium/state_trackers/nine/nine_ff.c
> > > > ../src/gallium/state_trackers/nine/nine_ff.c: In function 
> > > > ‘nine_ff_get_vs’:
> > > > ../src/gallium/state_trackers/nine/nine_ff.c:1610:72: error:
> > > > suggest 
> > > > braces around empty body in an ‘else’ statement [-Werror=empty-
> > > > body]
> > > >  1610 |                     DBG("FF given texture coordinate >=
> > > > 8. 
> > > > Ignoring\n");
> > > > >       ^
> > > > cc1: some warnings being treated as errors
> > > > 
> > > > Thanks,
> > > > Dieter
> > > > 
> > > 
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev



More information about the mesa-dev mailing list