[Mesa-dev] [PATCH 1/6] draw: silence unused var warnings for non-debug build

[Brian Paul]

On 06/01/2015 07:39 AM, Jose Fonseca wrote:
> On 01/06/15 14:20, Brian Paul wrote:
>> ---
>>   src/gallium/auxiliary/draw/draw_llvm.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c
>> b/src/gallium/auxiliary/draw/draw_llvm.c
>> index b9e55af..885c27c 100644
>> --- a/src/gallium/auxiliary/draw/draw_llvm.c
>> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
>> @@ -97,6 +97,7 @@ create_jit_dvbuffer_type(struct gallivm_state *gallivm,
>>      dvbuffer_type = LLVMStructTypeInContext(gallivm->context,
>> elem_types,
>>                                              Elements(elem_types), 0);
>>
>> +   (void) target; /* silence unused var warning for non-debug build */
>>      LP_CHECK_MEMBER_OFFSET(struct draw_vertex_buffer, map,
>>                             target, dvbuffer_type,
>>                             DRAW_JIT_DVBUFFER_MAP);
>> @@ -133,6 +134,7 @@ create_jit_texture_type(struct gallivm_state
>> *gallivm, const char *struct_name)
>>      texture_type = LLVMStructTypeInContext(gallivm->context, elem_types,
>>                                             Elements(elem_types), 0);
>>
>> +   (void) target; /* silence unused var warning for non-debug build */
>>      LP_CHECK_MEMBER_OFFSET(struct draw_jit_texture, width,
>>                             target, texture_type,
>>                             DRAW_JIT_TEXTURE_WIDTH);
>> @@ -290,6 +292,7 @@ create_gs_jit_context_type(struct gallivm_state
>> *gallivm,
>>      context_type = LLVMStructTypeInContext(gallivm->context, elem_types,
>>                                             Elements(elem_types), 0);
>>
>> +   (void) target; /* silence unused var warning for non-debug build */
>>      LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, constants,
>>                             target, context_type,
>> DRAW_GS_JIT_CTX_CONSTANTS);
>>      LP_CHECK_MEMBER_OFFSET(struct draw_gs_jit_context, num_constants,
>> @@ -353,6 +356,7 @@ create_jit_vertex_buffer_type(struct gallivm_state
>> *gallivm,
>>      vb_type = LLVMStructTypeInContext(gallivm->context, elem_types,
>>                                        Elements(elem_types), 0);
>>
>> +   (void) target; /* silence unused var warning for non-debug build */
>>      LP_CHECK_MEMBER_OFFSET(struct pipe_vertex_buffer, stride,
>>                             target, vb_type, 0);
>>      LP_CHECK_MEMBER_OFFSET(struct pipe_vertex_buffer, buffer_offset,
>>
>
>
> Series LGTM.
>
> Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
>
>
> The reasons these warnings are popping up is because we're using
> system's assert macro, instead of src/gallium/auxiliary/util/u_debug.h
>
> src/gallium/auxiliary/util/u_debug.h have two advantages over system's
> assert macro:
>
> 1. it avoids these unused variable warnings because the non-debug
> version includes the expression:
>
>    #define debug_assert(expr) (void)(0 && (expr))
>
> 2. On windows it will dump the assert failure message as
> stderr/OutputDebugMessage (instead of a MessageBox.)
>
> That said, I agree that trying to override system's assert is
> unsustainable.
>
> Your series addresses issue 1.  For issue 2, it might be worthwhile to
> override the _assert/_wassert built-in implementations instead.  This is
> the approach I recently did on Apitrace and it seems to work just as well:
>
>    https://github.com/apitrace/apitrace/blob/master/wrappers/assert.cpp

Looks interesting.


> This is not urgent.  But if you find yourself wondering why assertion
> failure messages don't appear on logs,  it might be worth doing it.

Yeah, the problem is when you include some sequence of header files, 
it's not clear which assert you're getting.  If you include u_debug.h 
first but then the system assert is included (indirectly) by some other 
header later, the system assert "wins".

My local hack has been to create a u_assert.h and make sure it's 
included last.  It contains just a few relevant lines from u_debug.h.

-Brian