[Mesa-dev] [PATCH v1 2/7] etnaviv: Add return statement to etna_amode so compiler is happy

[Chad Versace]

On Fri 16 Jun 2017, Christian Gmeiner wrote:
> 2017-06-16 14:54 GMT+02:00 Emil Velikov <emil.l.velikov at gmail.com>:
> > On 15 June 2017 at 21:47, Robert Foss <robert.foss at collabora.com> wrote:
> >> From: Tomeu Vizoso <tomeu.vizoso at collabora.com>
> >>
> >> Signed-off-by: Robert Foss <robert.foss at collabora.com>
> >> ---
> >>  src/gallium/drivers/etnaviv/etnaviv_compiler.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/src/gallium/drivers/etnaviv/etnaviv_compiler.c b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> >> index eafb511bb8..8f73113059 100644
> >> --- a/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> >> +++ b/src/gallium/drivers/etnaviv/etnaviv_compiler.c
> >> @@ -885,6 +885,8 @@ etna_amode(struct tgsi_ind_register indirect)
> >>     default:
> >>        assert(!"Invalid swizzle");
> >>     }
> >> +
> >> +   return 0;
> > All the cases are handled correctly and even the default one will
> > never be reached.
> > Guess the compiler isn't smart enough, since we're using int:2 while
> > in reality we're storing an enum and using enum:2 might not always
> > work.
> >
> > Alternative solutions is s/assert/unreachable/. The call will be up-to
> > the etna devs, but including something like the above (in the commit
> > summary or elsewhere) will be a good idea, IMHO.
> >
> 
> I really like Emil's idea of s/assert/unreachable/

If you replace assert() with unreachable() for an exhaustive switch
statement, it's best to delete the default case and call unreachable()
*after* the switch statement. That way, if new enums are ever added, the
compiler will emit a -Wswitch warning (which would be otherwise
supressed by the default case); someone will hopefully notice the
warning, fix it, and save users from an out-of-bounds instruction
pointer.