[Mesa-dev] [PATCH 2/2] glsl: Rewrote _mesa_glsl_process_extension to use table-driven logic.

[Paul Berry]

On 28 June 2011 10:01, Eric Anholt <eric at anholt.net> wrote:
> If I stumbled on this code, I'd have no idea what was going on.  A short
> version of this description near the code might help us poor C
> developers who stumble on it that have never seen this stuff before.

Will do.

>
>> >> +   case vertex_shader:   if (!this->avail_in_VS) return false; break;
>> >> +   case geometry_shader: if (!this->avail_in_GS) return false; break;
>> >> +   case fragment_shader: if (!this->avail_in_FS) return false; break;
>> >
>> > Delete the spurious breaks.
>>
>> Geez, what was I smoking?
>
> They don't look spurious in the code quoted here.  But they do look like
> some more whitespace would help a lot.

Aha, you're right! What I was smoking was
code-that-is-correct-but-not-obviously-so.  Had I not tried to format
it so compactly it would have been clear why the breaks are necessary:

case vertex_shader:
  if (!this->avail_in_VS) {
    return false;
  }
  break;
...etc.

Lesson learned: my code was so terse that I forgot why it was correct
when reading Ian's comments.

>
>>
>> >> +   for (unsigned int i = 0; i < Elements(_mesa_glsl_supported_extensions);
>> >> +        ++i) {
>> >
>> > Just 'unsigned'.
>>
>> Really?  I'm surprised you care about this detail, esp. considering
>> that there are many instances of "unsigned int" already in Mesa.  But
>> I'll acquiesce, especially since I'm trying to talk you into letting
>> me use a little-known C++ feature above :)
>
> I personally always preferred unsigned int to unsigned. :)
>

Me too, but I'll try not to fan any flames :)