[Mesa-dev] [PATCH] glsl: add default switch case to silence uninitialized variable warning
--- src/glsl/ast_to_hir.cpp |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index feff586..24efec6 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -1933,6 +1933,9 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state, case fragment_shader: limit = ctx->Const.FragmentProgram.MaxTextureImageUnits; break; + default: + limit = 0; + assert(!"Unexpected shader type"); } if (max_index >= limit) { -- 1.7.10.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: add default switch case to silence uninitialized variable warning
On Fri, Aug 23, 2013 at 1:24 PM, Brian Paul wrote: > --- > src/glsl/ast_to_hir.cpp |3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp > index feff586..24efec6 100644 > --- a/src/glsl/ast_to_hir.cpp > +++ b/src/glsl/ast_to_hir.cpp > @@ -1933,6 +1933,9 @@ validate_binding_qualifier(struct > _mesa_glsl_parse_state *state, >case fragment_shader: > limit = ctx->Const.FragmentProgram.MaxTextureImageUnits; > break; > + default: > + limit = 0; > + assert(!"Unexpected shader type"); >} > >if (max_index >= limit) { > -- > 1.7.10.4 I can't see why this would be necessary, since the switch statement switches over a variable of type _mesa_glsl_parser_targets, and the switch has cases for all elements in the enum. enum _mesa_glsl_parser_targets { vertex_shader, geometry_shader, fragment_shader }; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: add default switch case to silence uninitialized variable warning
On 08/23/2013 02:39 PM, Matt Turner wrote: On Fri, Aug 23, 2013 at 1:24 PM, Brian Paul wrote: --- src/glsl/ast_to_hir.cpp |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp index feff586..24efec6 100644 --- a/src/glsl/ast_to_hir.cpp +++ b/src/glsl/ast_to_hir.cpp @@ -1933,6 +1933,9 @@ validate_binding_qualifier(struct _mesa_glsl_parse_state *state, case fragment_shader: limit = ctx->Const.FragmentProgram.MaxTextureImageUnits; break; + default: + limit = 0; + assert(!"Unexpected shader type"); } if (max_index >= limit) { -- 1.7.10.4 I can't see why this would be necessary, since the switch statement switches over a variable of type _mesa_glsl_parser_targets, and the switch has cases for all elements in the enum. enum _mesa_glsl_parser_targets { vertex_shader, geometry_shader, fragment_shader }; gcc 4.8.1 on Fedora 19: ../../../src/glsl/ast_to_hir.cpp: In function 'void apply_type_qualifier_to_variable(const ast_type_qualifier*, ir_variable*, _mesa_glsl_parse_state*, YYLTYPE*, bool, bool)': ../../../src/glsl/ast_to_hir.cpp:1917:66: warning: 'limit' may be used uninitialized in this function [-Wmaybe-uninitialized] "(%d)", qual->binding, elements, limit); ^ ../../../src/glsl/ast_to_hir.cpp:1901:16: note: 'limit' was declared here unsigned limit; ^ -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: add default switch case to silence uninitialized variable warning
On 23 August 2013 13:55, Brian Paul wrote: > On 08/23/2013 02:39 PM, Matt Turner wrote: > >> On Fri, Aug 23, 2013 at 1:24 PM, Brian Paul wrote: >> >>> --- >>> src/glsl/ast_to_hir.cpp |3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp >>> index feff586..24efec6 100644 >>> --- a/src/glsl/ast_to_hir.cpp >>> +++ b/src/glsl/ast_to_hir.cpp >>> @@ -1933,6 +1933,9 @@ validate_binding_qualifier(**struct >>> _mesa_glsl_parse_state *state, >>> case fragment_shader: >>>limit = ctx->Const.FragmentProgram.**MaxTextureImageUnits; >>>break; >>> + default: >>> + limit = 0; >>> + assert(!"Unexpected shader type"); >>> } >>> >>> if (max_index >= limit) { >>> -- >>> 1.7.10.4 >>> >> >> I can't see why this would be necessary, since the switch statement >> switches over a variable of type _mesa_glsl_parser_targets, and the >> switch has cases for all elements in the enum. >> >> enum _mesa_glsl_parser_targets { >> vertex_shader, >> geometry_shader, >> fragment_shader >> }; >> >> > gcc 4.8.1 on Fedora 19: > > ../../../src/glsl/ast_to_hir.**cpp: In function 'void > apply_type_qualifier_to_**variable(const ast_type_qualifier*, > ir_variable*, _mesa_glsl_parse_state*, YYLTYPE*, bool, bool)': > ../../../src/glsl/ast_to_hir.**cpp:1917:66: warning: 'limit' may be used > uninitialized in this function [-Wmaybe-uninitialized] >"(%d)", qual->binding, elements, limit); > ^ > ../../../src/glsl/ast_to_hir.**cpp:1901:16: note: 'limit' was declared > here >unsigned limit; > ^ > In that case, let's fix it by initializing limit to 0 at its declaration. That way we will get a (helpful) compiler warning in the inevitable future event that we add new values to _mesa_glsl_parser_targets. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] glsl: add default switch case to silence uninitialized variable warning
On 08/23/2013 02:05 PM, Paul Berry wrote: On 23 August 2013 13:55, Brian Paul wrote: On 08/23/2013 02:39 PM, Matt Turner wrote: On Fri, Aug 23, 2013 at 1:24 PM, Brian Paul wrote: In that case, let's fix it by initializing limit to 0 at its declaration. That way we will get a (helpful) compiler warning in the inevitable future event that we add new values to _mesa_glsl_parser_targets. I second Paul's advice here. Let's not add default cases to switch statements that suppose to handle all possible enum values. Otherwise, we may silence helpful compiler warnings in the future. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev