Review: message passing of closures, and bug fixes

Adam requested that setmessage/getmessage be made to work for closures, which were previously not allowed to be messages.

OK, so this does it.

Also note opportunistic fixing of two bugs: (1) in typecheck.cpp, compiler was unintentionally disallowing messages that were arrays. (2) in opmessage.cpp:161, subtle bug where Name is uniform, Val is varying, message being retrieved was varying, didn't properly set the 'varying' flag, resulting in only the first point's value being copied to Val properly.

[rene.limberger, 2010-01-13 18:48:44]

http://codereview.appspot.com/186135/diff/1/6
File src/liboslexec/opmessage.cpp (right):

http://codereview.appspot.com/186135/diff/1/6#newcode47
src/liboslexec/opmessage.cpp:47: // don't know anything about them.// for those
we allocate new closures
i really just want to test these review comments.... why are there 2 slashes
here?

[larrygritz, 2010-01-13 19:19:54]

http://codereview.appspot.com/186135/diff/1/6
File src/liboslexec/opmessage.cpp (right):

http://codereview.appspot.com/186135/diff/1/6#newcode47
src/liboslexec/opmessage.cpp:47: // don't know anything about them.// for those
we allocate new closures
On 2010/01/13 18:48:44, rene.limberger wrote:
> i really just want to test these review comments.... why are there 2 slashes
> here?

My bad.  I think just bad cut and paste while formatting the comment text?  I'll
fix before commit.

[ckulla, 2010-01-13 19:31:29]

http://codereview.appspot.com/186135/diff/1/6
File src/liboslexec/opmessage.cpp (right):

http://codereview.appspot.com/186135/diff/1/6#newcode77
src/liboslexec/opmessage.cpp:77: type = TypeDesc::TypeInt;     // Actually store
closure indices only
Does this mean a closure message might get confused with an int message of the
same name?

[larrygritz, 2010-01-13 19:43:33]

http://codereview.appspot.com/186135/diff/1/6
File src/liboslexec/opmessage.cpp (right):

http://codereview.appspot.com/186135/diff/1/6#newcode77
src/liboslexec/opmessage.cpp:77: type = TypeDesc::TypeInt;     // Actually store
closure indices only
On 2010/01/13 19:31:29, ckulla wrote:
> Does this mean a closure message might get confused with an int message of the
> same name?


Actually, yes.  I haven't figured out how to solve that yet, but at least we
check if it's a valid index before copying it.  I'll put in a FIXME comment and
try to come back to this.  

Hopefully in the real world it will be rare for somebody to setmessage a closure
color, and then getmessage a message with the same name but try to store the
results into an int, or vice versa.

[ckulla, 2010-01-13 19:47:06]

LGTM

Could you also add a test for the fixed array functionality?

[larrygritz, 2010-01-13 20:06:17]

On 2010/01/13 19:47:06, ckulla wrote:
> LGTM
> 
> Could you also add a test for the fixed array functionality?

Yes, will do after the commit and release.