Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
On Tue, Apr 30, 2013 at 1:52 AM, Kenneth Graunke <kenn...@whitecape.org> wrote: > Consider the following shader: > > vec4 f(vec4 v) { return v; } > vec4 f(vec4 v); > > The prototype exactly matches the signature of the earlier definition, > so there's absolutely no point in it. However, it doesn't appear to > be illegal. The GLSL 4.30 specification offers two relevant quotes: > > "If a function name is declared twice with the same parameter types, > then the return types and all qualifiers must also match, and it is the > same function being declared." > > "User-defined functions can have multiple declarations, but only one > definition." > > In this case the same function was declared twice, and there's only one > definition, which fits both pieces of text. There doesn't appear to be > any text saying late prototypes are illegal, so presumably it's valid. > > Unfortunately, it currently triggers an assertion failure: > ir_dereference_variable @ <p1> specifies undeclared variable `v' @ <p2> > > When we process the second line, we look for an existing exact match so > we can enforce the one-definition rule. We then leave sig set to that > existing function, and hit sig->replace_parameters(&hir_parameters), > unfortunately nuking our existing definition's parameters (which have > actual dereferences) with the prototype's bogus unused parameters. > > Simply bailing out and ignoring such late prototypes is the safest > thing to do. > > Fixes Piglit's late-proto.vert as well as 3DMark/Ice Storm for Android. > > NOTE: This is a candidate for stable branches. > Cc: Tapani Pälli <tapani.pa...@intel.com> > Cc: Ian Romanick <i...@freedesktop.org> > Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> > --- > src/glsl/ast_to_hir.cpp | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp > index 2638411..e595110 100644 > --- a/src/glsl/ast_to_hir.cpp > +++ b/src/glsl/ast_to_hir.cpp > @@ -3205,10 +3205,17 @@ ast_function::hir(exec_list *instructions, > "match prototype", name); > } > > - if (is_definition && sig->is_defined) { > - YYLTYPE loc = this->get_location(); > - > - _mesa_glsl_error(& loc, state, "function `%s' redefined", name); > + if (sig->is_defined) { > + if (is_definition) { > + YYLTYPE loc = this->get_location(); > + _mesa_glsl_error(& loc, state, "function `%s' redefined", > name); > + } else { > + /* We just encountered a prototype that exactly matches a > + * function that's already been defined. This is redundant, > + * and we should ignore it. > + */ > + return NULL; > + } > } > } > } else { > -- > 1.8.2.1 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev