sclang: protect keysValuesArrayDo primitive

[telephon]

This can happen in corner cases. Fixes #852.

[telephon]

It fails with a double error message:

ERROR: primitive failed: keysValuesArrayDo first argument should be an array.
ERROR: Primitive '_ObjectString' failed.
Failed.
RECEIVER:
Instance of Event {    (0x11a59da68, gc=74, fmt=00, flg=00, set=03)

But maybe someone can give me hints of how to improve it!

[mossheim]

Can this error message please match the others in this file? See above line 1666:

error("Integer-forBy : endval or stepval not an Integer.\n");

and below line 1967:

error("Number-forSeries : first, second or last not an Integer or Float.\n");

[mossheim]

Shouldn't this check happen before lines 1759-1760?

[mossheim]

That double error message matches what happens when hitting similar cases in PyrInterpreter3:

3.forBy("string"); 
// ERROR: Integer-forBy : endval or stepval not an Integer.
// ERROR: Primitive '_ObjectCompileString' failed.

3.forSeries("garbage");
// ERROR: Number-forSeries : first, second or last not an Integer or Float.
// ERROR: Primitive '_ObjectCompileString' failed.

I think it's fine for the purposes of this PR. Thanks for this!

[mossheim]

There's an inconsistency between the check (IsNil) and the error message (should be an array). One of those needs to be modified—I'd go for more robust type-checking at this point, but since the crash only happens when the input is nil, amending the error message wouldn't be bad either.

[telephon]

the check could be more elaborate, but given that this is an internal method, checking against nil is ok, I think. Alternatively, one can check how it's done in the array primitives - it is a little more complicated there.

[telephon]

I still find it weird that the second error message is ERROR: Primitive '_ObjectString' failed.. But, well.

[nhthn]

Can we get a unit test please?

[mossheim]

@telephon Can you give the code you used to produce that error message? I find it interesting that mine refers to the primitive _ObjectCompileString.

The code looks pretty good; I am OK to merge once there's a unit test. :)

[telephon]

().keysValuesArrayDo()

I'll do a test next week.

[telephon]

this code: e = (); e.array = nil; e.keysValuesDo({}); causes the interpreter go in an infinite print loop, repeating:

Instance of PrimitiveFailedError {    (0x11a441f88, gc=88, fmt=00, flg=00, set=03)
  instance variables [5]
    what : "Failed."
    protectedBacktrace : nil
    path : nil
    receiver : instance of Event (0x16bbc6fa8, size=5, set=3)
    failedPrimitiveName : Symbol '_ObjectString'
}
ERROR: Dictionary-keysValuesArrayDo: first argument should not be nil.
ERROR: throw during error handling!

[telephon]

I'm not sure if I'm doing the right thing to the stack here:

if (IsNil(&vars[1])) {
	error("Dictionary-keysValuesArrayDo: first argument should not be nil.\n");
	
	slotCopy(++sp, &g->receiver);
	numArgsPushed = 1;
	selector = gSpecialSelectors[opmPrimitiveFailed];
	slot = sp;
	
	goto class_lookup;
}

[telephon]

Ok, setting numArgsPushed = 0 fixes it. Please have a look if that is correct.

[telephon]

(can be merged IMHO)

[mossheim]

I was wary of this one mainly because of the uncertain copy-pasting into the main interpreter loop, but I guess since the others seem to be similarly weird with posting _ObjectCompileString / _ObjectString it's probably OK. One thing I noticed just now is that the unit tests test an Event object and not a Dictionary. I'm going to change that and then merge. Thanks!

[mossheim]

Thanks! Eventually we should get to the bottom of why _ObjectString is the named primitive in the error message/call stack.