[Mesa-dev] [PATCH] clover: Pass unquoted compiler arguments to Clang

[Francisco Jerez]

Vedran Miletić <vedran at miletic.net> writes:

> 06.06.2016 u 12:24, Vedran Miletić je napisao/la:
>> On 06/06/2016 02:04 AM, Francisco Jerez wrote:
>>> Vedran Miletić <vedran at miletic.net> writes:
>>>>
>>>> Aside from working just like NVIDIA and AMD proprietary stacks, no.
>>>>
>>>> However, what we do right now is most certainly broken. Consider the
>>>> following argument
>>>>
>>>> -I"/foo bar/baz"
>>>>
>>>> which is going to be sent to Clang as two arguments broken over space,
>>>> while it should be one.
>>>
>>> But the OpenCL spec doesn't impose any explicit requirements on the
>>> formatting of the option string (unless I'm missing something), so
>>> strictly speaking both interpretations are equally broken.  It wasn't my
>>> intention to argue against implementing the same behaviour as the
>>> proprietary CL stacks as temporary solution, but the fact that this is
>>> unspecified is certainly an OpenCL spec bug.
>>>
>>
>> Agree that the spec is incomplete. Additional problem is that Clang is
>> unable to handle the incorrectly split arguments, in which case Clang
>> IMHO does the correct thing. Mesa is right now breaking the user/app
>> supplied compiler args into an array before passing it to Clang and that
>> breaking is done in a wrong way.
>>
>> I am not claiming that it's Clover's job to unquote args, even though it
>> would be convenient for Clang if it was.
>>
>> Regards,
>> Vedran
>>
>>>> We should split arguments correctly, and whether or not we remove
>>>> quotes in the process or we expect Clang to handle them is the easier
>>>> part of the job.
>>>>
>>
>
> Francisco,
>
> I am rebasing this. Do you suggest to replace the code in tokenize() or 
> to add an additional boolean argument to it, e.g. respect_quoting and 
> then do an if(respect_quoting) branching inside?
>

You can just replace the current implementation of tokenize(), it's not
used for anything else other than splitting compiler arguments AFAIK.

> Also, I would really like to progress on getting this merged in some 
> way, so any general feedback is also welcome.
>

You've got some more feedback in my previous reply it doesn't look like
you've taken into account yet, it would be nice to see those addressed
before we get your patch merged.

> Regards,
> Vedran
>
> -- 
> Vedran Miletić
> vedran.miletic.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160712/6620956d/attachment.sig>