[Mesa-dev] [PATCH 1/5] darwin: Suppress type conversion warnings for GLhandleARB

[Julien Isorce]

On 19 June 2015 at 10:24, Jose Fonseca <jfonseca at vmware.com> wrote:

> On 19/06/15 04:46, Ian Romanick wrote:
>
>> On 06/17/2015 10:53 PM, Julien Isorce wrote:
>>
>>> From: Jon TURNEY <jon.turney at dronecode.org.uk>
>>>
>>> On darwin, GLhandleARB is defined as a void *, not the unsigned int it
>>> is on
>>> linux.
>>>
>>> For the moment, apply a cast to supress the warning
>>>
>>> Possibly this is safe, as for the mesa software renderer the shader
>>> program
>>> handle is not a real pointer, but a integer handle
>>>
>>> Probably this is not the right thing to do, and we should pay closer
>>> attention
>>> to how the GLhandlerARB type is used.
>>>
>>
>> In Mesa, glBindAttribLocation (which takes GLuint) and
>> glBindAttribLocationARB (which takes GLhandleARB) are *the same
>> function*.  The same applies to pretty much all the other GLhandleARB
>> functions.
>>
>
>
> Properly fixing this is a nightmare, but I think that short term
> workaround is feasible.
>
> This is the generated glapitemp.h:
>
>   KEYWORD1 void KEYWORD2 NAME(BindAttribLocationARB)(GLhandleARB program,
> GLuint index, const GLcharARB * name)
>   {
>       (void) program; (void) index; (void) name;
>      DISPATCH(BindAttribLocation, (program, index, name), (F,
> "glBindAttribLocationARB(%d, %d, %p);\n", program, index, (const void *)
> name));
>   }
>
> Provided that GLhandlerARB is defined as `unsigned long` during Mesa build
> on MacOSX


Hi, where exactly ? or do you mean we just need to apply the patch [1] you
pointed ?


> (to avoid these int<->void *) conversions [1], the compiler should
> implicitly cast the 64bits GLhandlerARB program to an 32-bits GLuint.
>
> So, when an app calls glBindAttribLocationARB it will be dispatched to
> _mesa_BindAttribLocation, and the program truncated. So it should all just
> work.
>
> Ditto for when GLhandleARB appears as return value.
>
>
> The only problem is when GLhandleARB appears as a pointer, as there is
> only one such instance:
>
>   GLAPI void APIENTRY glGetAttachedObjectsARB (GLhandleARB containerObj,
> GLsizei maxCount, GLsizei *count, GLhandleARB *obj);
>
> But we do have a separate entry-point for this
> (_mesa_GetAttachedObjectsARB) so again, we're all good.
>
>
> So, Jon/Julien's patch seems perfectly workable -- all really left to do
> is to silence  GLhandleARB <-> GLuint conversions.
>

That's a good news.
So Jose concretely what needs to be done ? Just apply the patch [1] you
pointed or apply cast everywhere ?

All conversions are in the 3 files, src/mesa/main/dlist.c,
src/mesa/main/shaderapi.c
and src/mesa/main/shader_query.cpp, am I right ?

I did not notice before, but without any change on upstream code, it gives
an error only when compiling c++ files. For c files it is a warning:

main/shaderapi.*c*:1148:23: warning: incompatible pointer to integer
conversion passing 'GLhandleARB' (aka 'void *') to parameter of type
'GLuint' (aka 'unsigned int') [-Wint-conversion] attach_shader(ctx,
program, shader);

main/shader_query.*cpp*:72:7: error: no matching function for call to
'_mesa_lookup_shader_program_err'
"glBindAttribLocation");../../src/mesa/main/shaderobj.h:89:1:
note: candidate function not viable: cannot convert argument of incomplete
type 'GLhandleARB' (aka 'void *') to 'GLuint' (aka 'unsigned
int')_mesa_lookup_shader_program_err(struct
gl_context *ctx, GLuint name,

Thx

>
>
> Jose
>
>
> [1] Apitrace also defines GLhandleARB as unsigned long internally to avoid
> this
> https://github.com/apitrace/apitrace/blob/master/thirdparty/khronos/GL/glext.patch
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150625/4b9801ea/attachment.html>