This is tough question, I thought about it some time and concluded that some
function class must include body range, otherwise there will be no easy way to
get function end (it contents may be iterated (I did it in my code before I
realized I need to change locations placing itself, it was ugly), but then end
will point at last element end, not actual body end, which may be separated far
away by spaces and newlines). Function header already have range for name, if
it required, and only reasonable place to do it is here. I think additional
check needed for what lexical errors really may occurs here to throw message if
any. Anything I can think of now is prototype errors either components with
their own locations.
On Wed, 05 Feb 2014 12:41:58 -0800
Carl Worth <cwo...@cworth.org> wrote:
> Sir Anthony <anth...@adsorbtion.org> writes:
>
> Thanks for this patch series. It looks great to me. I sent a separate
> email with a small comment about some comments. The only other review
> comment I have is:
>
> > @@ -2127,7 +2138,7 @@ function_definition:
> > {
> > void *ctx = state;
> > $$ = new(ctx) ast_function_definition();
> > - $$->set_location(yylloc);
> > + $$->set_location_range(@1, @2);
> > $$->prototype = $1;
> > $$->body = $2;
>
> Doesn't this set the range for this definition to the entire range of
> the function (declaration and body)? In one sense, that's a correct
> range for what is being parsed here. But on the other hand, would this
> actually be a useful range to emit in an error message? Or would it be
> better to just direct the error message to the location of the prototype
> itself?
>
> I don't have strong feelings one way or the other, but just wanted to
> raise this issue to see if this is what you actually intended in this
> case.
>
> Regardless of whether you restrict the range in this case, the entire
> series is:
>
> Reviewed-by: Carl Worth <cwo...@cworth.org>
>
> -Carl
>
> --
> carl.d.wo...@intel.com
--
Sir Anthony <anth...@adsorbtion.org>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
