Sleepy __len__.

[albertjan]

I know it's absolutely ridiculous, but you brought it up @meredydd.

This didn't work:

import time

class Test():

    def __len__(self):
        time.sleep(1)
        return 42

t = Test()
print len(t)

This PR fixes that 😄 Although I don't know if this is correct.

[meredydd]

a) Not necessarily ridiculous; I think have code that looks very much like this and given that your test fails, I'm somewhat surprised it works!

b) I'm not sure this fix is quite safe. The whole point of the little hack here is that the skuplt-named functions (sq$length etc) called in circumstances that do not permit suspension - and in those cases, we need to call callsim or we will unpredictably break Javascript calling code if the dunder-named Python function (__len__) suspends. (Instead, it should break loudly with a "cannot suspend here", which is what callsim does.) The point was that the sq$length function gets created with a canSuspend argument, that governs whether it suspends or not.

Therefore, I suspect that the bug you're encountering is actually that sq$length is getting called without the parameter that tells it that it may suspend. (That, or there's a bug in my code, and it's getting called with true as its canSuspend argument, but the generated function isn't picking it up.)

[albertjan]

On line 55 of type.js

"__len__": ["sq$length", 0]

which causes this check to fail and go to the callsim branch: 0 is false

if (canSuspendIdx) {

But when I change this to if (canSuspendIdx === null) { it splices off the self argument for the call to __len__ here: args.splice(canSuspendIdx+1, 1); because the args array doesn't contain a canSuspend argument.

[meredydd]

Excellent. So that code was, in fact, entirely broken, and somehow my testing failed to capture it. Go me.

Where is the self argument, then? That's what the +1 was theoretically meant to account for...

[albertjan]

So it expects to get the canSuspend argument from the args array but it isn't there so instead of splicing that off it splices the self argument.

[rixner]

I think the check just needs to be changed to:

if (canSuspendIdx !== null) {

@meredydd, I think that's what you were trying to check for, but you passed an index of 0 (legitimately), which evaluates to false. I didn't test this or anything, but looking at the code, I think this is at least what you meant.

[rixner]

I also think you need:

"__len__": ["tp$length", 0]

As this is what gets called from Sk.builtin.len, but this causes other things to fail.

[rixner]

@albertjan, unfortunately, I don't think you can do this. I'm sure there are places that call sq$length that can't handle it returning a suspension. If you look at Sk.builtin.len, it passes true to tp$length, because it can handle a suspension. I think you have to check for the caller passing true. @meredydd can obviously correct me if I'm wrong...

[albertjan]

3rd times a charm, the suspendIdx should not have been 0 because self is there canSuspend if there is at 1.

[meredydd]

LGTM

[albertjan]

@skulpt/committers merge at will 😄.