Squirrel

The following code produces a debug assertion in both Squirrel 2.2.2 stable and Squirrel 3.0 alpha 1:

function foo() {
   yield
}
foo.call(this) // Result: Assertion failed: _unVal.pGenerator, file c:\squirrel2\squirrel\sqobject.h, line 187

As far as I can tell, the problem appears to be in the "switch(et)" statement near the top of SQVM::Execute in sqvm.cpp.  The ET_CALL case is reached as a result of a .call() invocation, and it seems to not be taking into account the possibility that the function called is a generator function. As a result, no generator is created, and when Execute() hits the first OT_YIELD operation, it finds that ci->_generator is NULL and thus it thinks the closure being called is not a generator and tries to report the attempt to yield as an error. ("only generator can be yielded")

As it turns out, the assert failure only happens because it also tries to print the typename of ci->_generator... which is of course NULL. :-P

So, my thinking is that it should probably check for a generator function in the ET_CALL case, and actually create an SQGenerator after calling StartCall(). I'm not really sure exactly how it would work yet, though. Still trying to plod through and figure this stuff out. :-P

Yes, as is now a generator cannot be 'called', It shouldn't throw an exception though. I'm on it.

thx

Alberto

Hm, actually, I think I may have fixed it! (Emphasis on the may.)

After spending a while getting acquainted with the calling procedure and SQVM::Execute, I managed to get generator functions working with .call()/.acall()/etc. It's a relatively small code change, but I'm still not completely familiar with Squirrel's innards, so there may be some unintended problems caused by this code. It seems to be working for me, but I'd appreciate some feedback from those who know better. :-)

Basically, just change the entire ET_CALL case of the switch at the top of SQVM::Execute to the following:

case ET_CALL:{
     SQInteger last_top = _top;
     if(!StartCall(_closure(closure), _top - nargs, nargs, stackbase, false)) {
         //call the handler if there are no calls in the stack, if not relies on the previous node
         if(ci == NULL) CallErrorHandler(_lasterror);
         return false;
     }
     // generator.call() bugfix:
     if(_funcproto(_closure(closure)->_function)->_bgenerator) {
         SQGenerator *gen = SQGenerator::Create(_ss(this), _closure(closure));
         _GUARD(gen->Yield(this));
         POP_CALLINFO(this);
         outres = SQObjectPtr(gen);
         while (last_top >= _top) _stack._vals[last_top--].Null();
         return true;
     }
     ci->_root = SQTrue;
     }
     break;

Then .call(), .acall(), .pcall(), and .pacall() should work correctly on generator functions. (At least, as far as I can tell...) Alternately, I've uploaded a fixed copy of sqvm.cpp from Squirrel 2.2.2 stable if anybody wants to just drop it in and try it out. I also wrote up a quick .nut script to test using .call() methods on generator functions. I haven't tried this in 3.0 alpha 1 yet, but I imagine it should work there also.


Also, I found another bug, possibly related to the first one. Initially, I figured that I could just emulate .call() on a generator by wrapping the generator call in another function and using .call() on that. Unfortunately, this also crashes Squirrel:

function gen() {
   print(this)
   yield
}
function wrapper() {
   return gen()
}
local g = wrapper.call("mycontext") // immediate crash

Oddly enough, this only seems to cause a crash when directly returning the result of a generator function evaluation from a method invoked with .call(). If the method is invoked normally, or if you store the generator in a local variable before returning it, everything works perfectly. For example:

function gen() {
   print(this)
   yield
}
function wrapper() {
   local g = gen()
   return g
}
local g = wrapper.call("mycontext") // this works fine

Seems like an odd and somewhat obscure bug. Definitely seems to be a different cause than the first bug though, from a cursory investigation.

Your fix/implementation is correct, everything should work just fine. The crash on the return is due to the fact that is a tailcall, while if you store it in a variable is not. Simply is not possible to safely invoke a generator as a tail call in certain cases, the solution is checking if it is a generator and transform the tailcall in a normal call.

case _OP_TAILCALL:
 temp_reg = STK(arg1);
 if (type(temp_reg) == OT_CLOSURE
  && !_funcproto(_closure(temp_reg)->_function)->_bgenerator){
...
...
...

this should fix it

Alberto

 

Ah, that makes sense. That works perfectly, thanks!