codefrau / SqueakJS

A Squeak Smalltalk VM in Javascript
https://squeak.js.org
MIT License
365 stars 75 forks source link

Primitive 100 should have two variants #93

Closed pavel-krivanek closed 4 years ago

pavel-krivanek commented 4 years ago

There are two possible calling conventions of the <primitive 100> that have different arguments count:

Object>>#perform:withArguments:inSuperclass:
Context>>#object:perform:withArguments:inClass: (ContextPart)

The SqueakJS knows only the first one

pavel-krivanek commented 4 years ago

VERY NAIVE and messy hotfix:

        primitivePerformWithArgs: function(argCount, supered) {
           if (argCount !== 4)
           {
                var rcvr = this.stackValue(argCount);
                var selector = this.stackValue(argCount - 1);
                var args = this.stackValue(argCount - 2);
                if (args.sqClass !== this.specialObjects[Squeak.splOb_ClassArray])
                    return false;
                var lookupClass = supered ? this.stackValue(argCount - 3) : this.getClass(rcvr);
                if (supered) { // verify that lookupClass is in fact in superclass chain of receiver;
                    var cls = this.getClass(rcvr);
                    while (cls !== lookupClass) {
                        cls = cls.pointers[Squeak.Class_superclass];
                        if (cls.isNil) return false;
                    }
                }
                var trueArgCount = args.pointersSize();
                var stack = this.activeContext.pointers;
                var trueArgCount = args.pointersSize();
                var trueArgStart = supered ? this.sp - 2 : this.sp - 1;
                var stack = this.activeContext.pointers;
                this.arrayCopy(args.pointers, 0, stack, trueArgStart, trueArgCount);
                this.sp += trueArgCount - argCount; //pop selector and array then push args
                var entry = this.findSelectorInClass(selector, trueArgCount, lookupClass);
                this.executeNewMethod(rcvr, entry.method, entry.argCount, entry.primIndex, entry.mClass, selector);
                return true;
            } else {
                var rcvr = this.stackValue(argCount-1);
                var selector = this.stackValue(argCount - 2);
                var args = this.stackValue(argCount - 3);
                if (args.sqClass !== this.specialObjects[Squeak.splOb_ClassArray])
                    return false;
                var lookupClass = this.stackValue(argCount - 4);
                if (supered) { // verify that lookupClass is in fact in superclass chain of receiver;
                    var cls = this.getClass(rcvr);
                    while (cls !== lookupClass) {
                        cls = cls.pointers[Squeak.Class_superclass];
                        if (cls.isNil) return false;
                    }
                }
                var trueArgCount = args.pointersSize();
                var stack = this.activeContext.pointers;
                var trueArgCount = args.pointersSize();
                var trueArgStart = supered ? this.sp - 2 : this.sp - 1;
                var stack = this.activeContext.pointers;
                this.push(rcvr);
                this.arrayCopy(args.pointers, 0, stack, trueArgStart, trueArgCount);
                this.sp += trueArgCount - argCount; //pop selector and array then push args
                var entry = this.findSelectorInClass(selector, trueArgCount, lookupClass);
                this.executeNewMethod(rcvr, entry.method, entry.argCount, entry.primIndex, entry.mClass, selector);
                return true;
            }
        },

We need to use different argument indexes, use the last argument as lookupClass, push target receiver on the stack too.

ccrraaiigg commented 4 years ago

I submitted a fix for the primitive that accommodates both conventions, in https://github.com/bertfreudenberg/SqueakJS/pull/90/.

pavel-krivanek commented 4 years ago

hmm, your fix was integrated (in a different form) and this fix started from it. But this primitive was still not working correctly in Pharo where it was failing during inspector opening.

codefrau commented 4 years ago

The Right Fix for this is to recognize that the stack layout is the same for all 3 variants:

Object>>perform:withArguments:
      receiver - selector - args
      stack(2) - stack(1) - stack(0)
      argCount: 2, supered: false
Object>>perform:withArguments:inSuperclass:
      receiver - selector - args - class
      stack(3) - stack(2) - stack(1) - stack(0)
      argCount: 3, supered: true
Context>>object:perform:withArguments:inClass:
      object - selector - args - class
      stack(3) - stack(2) - stack(1) - stack(0)
      argCount: 4, supered: true

I was not aware of the 3rd variant when I wrote the original code, so I used stack(argCount) to get the receiver. A better way is to base it on the "supered" flag. Let me push a fix, please test.