On 23 December 2011 14:35, Ian Romanick <<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> From: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>> Somethings, like pre-increment operations, were not previously caught. After the 8.0 release, this code needs some major refactoring and clean-up. It's a mess. :( Signed-off-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com" target="_blank">ian.d.romanick@intel.com</a>> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=42755" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=42755</a> --- src/glsl/ast_function.cpp | 57 +++++++++++++++++++++++++++++++++++++++++--- 1 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp index 126b610..4471c76 100644 --- a/src/glsl/ast_function.cpp +++ b/src/glsl/ast_function.cpp @@ -96,6 +96,7 @@ prototype_string(const glsl_type *return_type, const char *name, static ir_rvalue * generate_call(exec_list *instructions, ir_function_signature *sig, YYLTYPE *loc, exec_list *actual_parameters, + ir_call **call_ir, struct _mesa_glsl_parse_state *state) </blockquote><div> Another minor suggestion: The semantics of call_ir are: - If generate_call is successful, it is set to the ir_call that is generated. - If an error occurred, it is unchanged. This means that in order for the caller to be robust in the case of error conditions, it must remember to set call_ir to NULL before calling this function. You're doing that, so there's no bug, but I'd feel better about the situation if we set *call_ir to NULL at the top of generate_call rather than in the caller. That way call_ir would behave like a true out parameter even in the case of error. As with my other comment, I'm not married to the idea; either way the series is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com" target="_blank">stereotype441@gmail.com</a>> </div> <blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> { void *ctx = state; @@ -256,10 +257,12 @@ generate_call(exec_list *instructions, ir_function_signature *sig, deref = new(ctx) ir_dereference_variable(var); ir_assignment *assign = new(ctx) ir_assignment(deref, call, NULL); instructions->push_tail(assign); + *call_ir = call; deref = new(ctx) ir_dereference_variable(var); } else { instructions->push_tail(call); + *call_ir = call; deref = NULL; } instructions->append_list(&post_call_conversions); @@ -269,6 +272,7 @@ generate_call(exec_list *instructions, ir_function_signature *sig, static ir_rvalue * match_function_by_name(exec_list *instructions, const char *name, YYLTYPE *loc, exec_list *actual_parameters, + ir_call **call_ir, struct _mesa_glsl_parse_state *state) { void *ctx = state; @@ -342,7 +346,8 @@ done: } /* Finally, generate a call instruction. */ - return generate_call(instructions, sig, loc, actual_parameters, state); + return generate_call(instructions, sig, loc, actual_parameters, + call_ir, state); } else { char *str = prototype_string(NULL, name, actual_parameters); @@ -1442,9 +1447,53 @@ ast_function_expression::hir(exec_list *instructions, process_parameters(instructions, &actual_parameters, &this->expressions, state); - return match_function_by_name(instructions, - id->primary_expression.identifier, & loc, - &actual_parameters, state); + ir_call *call = NULL; + ir_rvalue *const value = + match_function_by_name(instructions, + id->primary_expression.identifier, + &loc, &actual_parameters, &call, state); + + if (call != NULL) { + /* If a function was found, make sure that none of the 'out' or 'inout' + * parameters violate the extra l-value rules. + */ + ir_function_signature *f = call->get_callee(); + assert(f != NULL); + + exec_node *formal_node = f->parameters.head; + + foreach_list (actual_node, &this->expressions) { + /* Both parameter lists had better be the same length! + */ + assert(!actual_node->is_tail_sentinel()); + + const ir_variable *const formal_parameter = + (ir_variable *) formal_node; + const ast_expression *const actual_parameter = + exec_node_data(ast_expression, actual_node, link); + + if ((formal_parameter->mode == ir_var_out + || formal_parameter->mode == ir_var_inout) + && actual_parameter->non_lvalue_description != NULL) { + YYLTYPE loc = actual_parameter->get_location(); + + _mesa_glsl_error(&loc, state, + "function parameter '%s %s' references a %s", + (formal_parameter->mode == ir_var_out) + ? "out" : "inout", + formal_parameter->name, + actual_parameter->non_lvalue_description); + return ir_call::get_error_instruction(ctx); + } + + /* Only advance the formal_node pointer here because the + * foreach_list business already advances actual_node. + */ + formal_node = formal_node->next; + } + } + + return value; } return ir_call::get_error_instruction(ctx); -- 1.7.6.4 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>