[Mesa-dev] [PATCH] glcpp: Check version_resolved in the proper place.

[Matt Turner]

On Sat, Jan 25, 2014 at 12:57 PM, Carl Worth <cworth at cworth.org> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>> The check was in the wrong place, ...
>
> I don't doubt that the change here is good and correct, but I think
> there's likely some additional renaming that can be applied:
>
>> +     if (parser->version_resolved)
>> +             return;
>> +
>>       parser->version_resolved = true;
>
> Checking this field immediately before setting it clearly looks correct.
>
>>  glcpp_parser_resolve_version(glcpp_parser_t *parser)
>>  {
>> -     if (parser->version_resolved)
>> -             return;
>> -
>
> But it looks odd to not be checking the "version_resolved" field within
> the "resolve_version" function.
>
> I'm not looking at the actual code now in order to provide a
> recommendation, but perhaps it would be more clear if the field were
> renamed to be consistent with the naming of the function performing the
> guarded manipulation?
>
> -Carl

Right, that's a good suggestion. I'm not able to come up with a new
name for either the field or function that I'm terribly happy with.
I've renamed

glcpp_parser_resolve_version -> glcpp_parser_resolve_implicit_version

which hopefully makes it a bit clearer? Two new patches on the mailing list.