Reviewers: ,

Description:
In several string ops, I used a local var that just told whether any of
the arguments were varying, to determine whether to assign to just the
first element of Result, or all of them. I should really have been
checking whether Result itself was varying. They could differ because
the adjust_varying(Result) will make it varying even if none of the args
are varying, if the instruction is itself inside a varying conditional.


Please review this at http://codereview.appspot.com/198076/show

Affected files:
src/liboslexec/opstring.cpp


Index: src/liboslexec/opstring.cpp
===================================================================
--- src/liboslexec/opstring.cpp (revision 563)
+++ src/liboslexec/opstring.cpp (working copy)
@@ -203,7 +203,7 @@
if (runflags[i]) {
result[i] = format_args (exec, format[i].c_str(),
nargs-2, args+2, i);
- if (! varying)
+ if (! Result.is_varying())
break;
}
}
@@ -232,7 +232,7 @@
if (runflags[i]) {
result[i] = ustring (format_args (exec, format.c_str(),
nargs-1, args+1, i));
- if (! varying)
+ if (! Result.is_varying())
break;
}
}
@@ -348,7 +348,7 @@
b = Imath::clamp (b, 0, (int)str.length());
int len = Imath::clamp (length[i], 0, (int)str.length());
result[i] = ustring (s[i], b, len);
- if (! varying)
+ if (! Result.is_varying())
break;
}
}
@@ -411,7 +411,7 @@
result[i] = fullmatch ? regex_match (subject[i].c_str(), *regex)
: regex_search (subject[i].c_str(), *regex);
}
- if (! varying)
+ if (! Result.is_varying())
break;
}
}

LGTM!

You might want to hunt for this pattern in a few other places.

get/setmessage come to mind

http://codereview.appspot.com/198076/show

I'm going to commit this ASAP and then hunt for other possible
occurrences.


http://codereview.appspot.com/198076/show