Review: error for type coercion of output params (issue204076)
larry...@...
Reviewers: ,
Description: Ordinarily, it's fine to have type coercion on function parameters, for example, if you pass a float where a color was expected, it'll just do the usual float-to-color conversion into a temporary before using it as an argument. But this is wrong if it's an output parameter -- because if the function *writes* to that coerced temporary, well, how the heck is the result supposed to get back to the original argument, which can't even hold a color? So this proposed change catches this error and prints as clear an error message as I could muster: {{{ test.osl:11: error: Cannot pass 'float f' as argument 2 to myfunc because it is an output parameter that must be a color }}} This is related to this issue: http://code.google.com/p/openshadinglanguage/issues/detail?id=51 Please review this at http://codereview.appspot.com/204076/show Affected files: src/liboslcomp/codegen.cpp Index: src/liboslcomp/codegen.cpp =================================================================== --- src/liboslcomp/codegen.cpp (revision 571) +++ src/liboslcomp/codegen.cpp (working copy) @@ -1111,6 +1111,7 @@ ASTNode *a = args().get(); int returnarg = !typespec().is_void(); + ASTNode *form = is_user_function() ? user_function()->formals().get() : NULL; for (int i = 0; a; a = a->nextptr(), ++i) { Symbol *thisarg = NULL; if (a->nodetype() == index_node && argwrite(i+returnarg)) { @@ -1128,9 +1129,23 @@ if (i < (int)polyargs.size() && polyargs[i].simpletype() != TypeDesc(TypeDesc::UNKNOWN) && polyargs[i].simpletype() != TypeDesc(TypeDesc::UNKNOWN, -1)) { + Symbol *origarg = thisarg; thisarg = coerce (thisarg, polyargs[i]); + // Error to type-coerce an output -- where would the result go? + if (thisarg != origarg && + ! equivalent (origarg->typespec(), form->typespec()) && + form && form->nodetype() == variable_declaration_node && + ((ASTvariable_declaration *)form)->is_output()) { + error ("Cannot pass '%s %s' as argument %d to %s\n\t" + "because it is an output parameter that must be a %s", + origarg->typespec().c_str(), origarg->name().c_str(), + i+1, user_function()->func()->name().c_str(), + form->typespec().c_str()); + } } argdest.push_back (thisarg); + if (form) + form = form->nextptr(); } if (is_user_function ()) {
|
|