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 ()) {

Join osl-dev@lists.aswf.io to automatically receive all group messages.