[Mesa-dev] [PATCH] glsl: do not show locp information if it is not available

[Ian Romanick]

On 10/31/2016 07:06 PM, Timothy Arceri wrote:
> On Mon, 2016-10-31 at 16:51 +0100, Juan A. Suarez Romero wrote:
>> On Wed, 2016-10-26 at 13:42 +0200, Juan A. Suarez Romero wrote:
>>>
>>> Ignore source file, line number and column in glcpp_error() and
>>> glcpp_warning() if those are not available.
>>>
>>> It fixes 4 piglit tests:
>>>   spec/glsl-1.10/compiler/version-0.frag: crash pass
>>>   spec/glsl-1.10/compiler/version-0.vert: crash pass
>>>   spec/glsl-es-3.00/compiler/version-0.frag: crash pass
>>>   spec/glsl-es-3.00/compiler/version-0.vert: crash pass
>>>
>>> V2:
>>>  - Use brackets (Timothy)
>>> ---
>>>  src/compiler/glsl/glcpp/pp.c | 43 +++++++++++++++++++++++++++++---
>>> --
>>> ---------
>>>  1 file changed, 29 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/src/compiler/glsl/glcpp/pp.c
>>> b/src/compiler/glsl/glcpp/pp.c
>>> index b591279..ab6a214 100644
>>> --- a/src/compiler/glsl/glcpp/pp.c
>>> +++ b/src/compiler/glsl/glcpp/pp.c
>>> @@ -32,13 +32,21 @@ glcpp_error (YYLTYPE *locp, glcpp_parser_t
>>> *parser, const char *fmt, ...)
>>>  	va_list ap;
>>>  
>>>  	parser->error = 1;
>>> -	ralloc_asprintf_rewrite_tail(&parser->info_log,
>>> -				     &parser->info_log_length,
>>> -				     "%u:%u(%u): "
>>> -				     "preprocessor error: ",
>>> -				     locp->source,
>>> -				     locp->first_line,
>>> -				     locp->first_column);
>>> +
>>> +        if (locp) {
>>> +           ralloc_asprintf_rewrite_tail(&parser->info_log,
>>> +                                        &parser->info_log_length,
>>> +                                        "%u:%u(%u): "
>>> +                                        "preprocessor error: ",
>>> +                                        locp->source,
>>> +                                        locp->first_line,
>>> +                                        locp->first_column);
>>> +        } else {
>>> +           ralloc_asprintf_rewrite_tail(&parser->info_log,
>>> +                                        &parser->info_log_length,
>>> +                                        "preprocessor error: ");
>>> +        }
>>> +
>>>  	va_start(ap, fmt);
>>>  	ralloc_vasprintf_rewrite_tail(&parser->info_log,
>>>  				      &parser->info_log_length,
>>> @@ -53,13 +61,20 @@ glcpp_warning (YYLTYPE *locp, glcpp_parser_t
>>> *parser, const char *fmt, ...)
>>>  {
>>>  	va_list ap;
>>>  
>>> -	ralloc_asprintf_rewrite_tail(&parser->info_log,
>>> -				     &parser->info_log_length,
>>> -				     "%u:%u(%u): "
>>> -				     "preprocessor warning: ",
>>> -				     locp->source,
>>> -				     locp->first_line,
>>> -				     locp->first_column);
>>> +        if (locp) {
>>> +           ralloc_asprintf_rewrite_tail(&parser->info_log,
>>> +                                        &parser->info_log_length,
>>> +                                        "%u:%u(%u): "
>>> +                                        "preprocessor warning: ",
>>> +                                        locp->source,
>>> +                                        locp->first_line,
>>> +                                        locp->first_column);
>>> +        } else {
>>> +           ralloc_asprintf_rewrite_tail(&parser->info_log,
>>> +                                        &parser->info_log_length,
>>> +                                        "preprocesor warning: ");
>>> +        }
>>> +
>>>  	va_start(ap, fmt);
>>>  	ralloc_vasprintf_rewrite_tail(&parser->info_log,
>>>  				      &parser->info_log_length,
>>
>>
>> Timothy, can this be considered as Reviewed-by: you?
> 
> I was hoping someone else would comment as I wasn't sure this was the
> right fix.
> 
> I'm not sure just showing "preprocessor error: " is the right solution.
> However I haven't looked into it enough to suggest anything else.
> 
> What do others think?

I was thinking the same thing.  Maybe just use 0 for all the values?
That way the formatting will at least match.

I'm also a little curious how we get to this point with locp being NULL.
 Some commentary about that should at least go in the commit message.

>> Thanks in advance!
>>
>> 	J.A.
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev