SCLang: ClassLib: Implement doesNotUnderstandAbout preserving kwargs across doesNotUnderstand calls

[JordanHendersonMusic]

Purpose and Motivation

When calling an unknown method without keyword args the supercollider vm passes all the information back into the language allowing for transparent wrapper classes. This is not the case with keyword arguments whose data is lost.

This PR fixes that issue by implementing a new method in Object.sc doesNotUnderstandAbout {|selector, argumentsArray, keywordArgumentsPairs|..}, creating methods to make using this easier, updating the objectprototyping mechanism, and updating the rest of the class library

A huge thank you to @telephon who has been invaluable in designing the class library methods and the help documentation!

Fixes #6064.
See #6227 for the messy history!

This PR has four commits, 1 and 2 are functionality, 3 and 4 actually do something useful for the end user — the latter two also serve as an example of usage.

1 SCLang: ClassLib: implement doesNotUnderstandAbout.

This commit contains all the core mechanisms of doesNotUnderstandAbout. By itself this commit is not that useful as it is hard to actually use the new method.

• SCLang: create doesNotUnderstandAbout.
• DoesNotUnderstandError: add an additional argument, allowing the keyword pairs to be passed through.
• CPP: change the existing cpp method doesNotUnderstandWithKeys to call the sclang method doesNotUnderstandAbout by wrapping the non-keyword arguments and keyword arguments in separate arrays, if there are no keywords, pass nil.

Exactly what method gets called when something isn't understood a a little complex to preserve backwards compatibility.

1. First the object's doesNotUnderstandAbout method will be tested to see if it is the same as Objects, if it isn't, then doesNotUnderstandAbout is called.
2. If there is a custom implementation of doesNotUnderstand, this is called, if there are keyword arguments in the call, a warning is posted.
3. Object's doesNotUnderstandAbout is called.

Object's doesNotUnderstand is never called by this mechanism. It is preserved and for the cases where the user manually calls the method. Object's implementation delegates to doesNotUnderstandAbout.

However... if a base class has implement doesNotUnderstand, it will called and the keywords discarded.
Additionally, if a child of that base implements doesNotUnderstandAbout, then only when there are no keywords will the base class's doesNotUnderstand method be called. This is confusing, but necessary for backwards compatibility.

2 ClassLib: implement performWith, valueWith and functionPerformWith along with error checking in Method/FunctionDef.

Implement performWith, valueWith, and functionPerformWith which allow an easy way to perform/value/functionPerform with both keyword arguments and normal arguments. This also centralises the error checking in Method/FunctionDef so that you can't call a method with overlapping arguments and keyword-arguments.

3 ClassLib: make objectPrototyping accept keywords.

Fix object prototyping so that keywords are passed to the function. Given the previous two commits, this is a small change.

4 ClassLib: replace calls to doesNotUnderstand with doesNotUnderstandAbout

There are only a few places where doesNotUnderstand was used in the class library, this replaces the majority of them.

Types of changes

• Documentation
• Bug fix
• New feature

To-do list

• Code is tested
• All tests are passing
• Updated documentation
• This PR is ready for review

[telephon]

To me, "deprecate" is too strong. Perhaps we should just describe it as a choice. Maybe I feel emotionally attached to the message doesNotUnderstand!

[telephon]

OK, I think we have to live with two versions of doesNotUnderstand.

I have two suggestions concerning naming.

1. keeping the smalltalk flavour of our language, let's call the new method doesNotUnderstandAbout. It works well with the argument names: doesNotUnderstandAbout(selector, argNames, keywordArgumentNames)
2. performWith is too close to flopWith (and haskell's foldWith), which all assume a function as a first argument. Here we can be more explicit and call the interface: performWithKeys, valueWithKeys, functionPerformWithKeys. It is more "technical", but that fits with the arguments.

What do you think from a native speaker's point of view?

[telephon]

But I am still thinking about how we could just do it with doesNotUnderstand. If you have an idea, drop it in!

[JordanHendersonMusic]

@telephon

I like the name doesNotUnderstandAbout! I chose doesNotUnderstandWithKeys as it is symmetrical with the c++ method. I'll change the the sc method to *About, but not the c++ as the split between keys and non-keys makes sense there.

---

performWith is too close to flopWith (and haskell's foldWith), which all assume a function as a first argument. Here we can be more explicit and call the interface: performWithKeys, valueWithKeys, functionPerformWithKeys. It is more "technical", but that fits with the arguments.

I don't think what Haskell does is important here, the clash with flopWith is problematic though. However, your name suggestions might imply that it only works with keys, like performKeyValuePairs. Ideally, these should just be called perform and the existing perform renamed to performWithOutKeywords. I really can't think of a better name.

---

I don't think it will be ever be possible to, without breaking anything, extended doesNotUnderstand because of the variable argument. After working on this, it seems variable arguments are quite problematic, I wonder if there is any guidelines on including them in future methods? I'd be of the opinion that they shouldn't be used in the class library unless it is the only argument and just passes the arguments along.

Maintaining two methods that do almost but not quite the same thing seems like a recipe for trouble, and creating the possibility that the code might do something different depending on whether keyword arguments are passed or not also seems like it would cause issues down the line.

For those two reasosn I do think that doesNotUnderstand should be 'deprecated', or at least, have all classes in the class library migrate to doesNotUnderstandAbout, and be discouraged from use.

---

if we want to make it easy for people to support the interface, Object should delegate: this.doesNotUnderstandWithKeys(selector, args).
Then if someone changes their interface to implement only doesNotUnderstandWithKeys, it will still work for their subclasses, if they have not yet updated the implementation.

I don't quite understand here because of how the a method call that isn't understood is resolved to what ever method the user has defined.

---

A little ambiguity that I noticed just now...

DNUWK_Base {
}

DNUWK_Child : DNUWK_Base {
	doesNotUnderstandWithKeys { |selector, argumentsArray, keywordArgumentPairs|
		^\child;
	}
}

DNUWK_Child().foo(\abc, 1) // returns \child
DNUWK_Child().foo(\abc, car: 1) // returns \child

This is all good. However, if doesNotUnderstand is implemented in the base class something potentially surprising happens.

DNUWK_Base {
	doesNotUnderstand { |selector ...args|
		^\base;
	}
}

DNUWK_Child : DNUWK_Base {
	doesNotUnderstandWithKeys { |selector, argumentsArray, keywordArgumentPairs|
		^\child
	}
}

DNUWK_Child().foo(\abc, 1) // returns \base
DNUWK_Child().foo(\abc, car: 1) // returns \child 

Here, implementing doesNotUnderstand in the base class changes the behaviour of the child class in a way that isn't all that obvious.

What do you think about making the default method be doesNotUnderstandWithKeys, and only if it doesn't exist, will doesNotUnderstand be attempted? Meaning, both the child method will be called in the above case?

[JordanHendersonMusic]

Ah, so I've just discovered something else when replacing IdentityDictionary's doesNotUnderstand with doesNotUnderstandAbout.

This call to superPerformList goes into infinite recursion.

// old
^this.superPerformList(\doesNotUnderstand, selector, args);
// new — this goes into an infinite loop
^this.superPerformList(\doesNotUnderstandAbout, selector, argumentsArray, keywordArgumentPairs)

To fix it, you have to do this...

^this.superPerformList(\doesNotUnderstandAbout, [selector, argumentsArray, keywordArgumentPairs])

This actually makes sense according to the help file, and (my understanding of) the c++, but I can't for the life of me figure out why it worked in the first place!?

int objectSuperPerformList(struct VMGlobals* g, int numArgsPushed) {
    ...

    // Here, if the argument isn't an object (an array of some kind) it just calls normal perform
    //      going into an infinite loop. 
    if (NotObj(listSlot)) {
        return objectPerform(g, numArgsPushed);
    }

    ...

    // It is only in the last few lines where the actually super object gets call.

    sendSuperMessage(g, selector, numArgsPushed);
    g->numpop = 0;
    return errNone;
}

[JordanHendersonMusic]

I've updated the pr to reflect these changes.

I haven't changed the names of performWith and the likes, but I am more than happy to if you still think it is necessary.

Hold off on looking at this though, I've just found a bug...

[JordanHendersonMusic]

Okay here is the current bug, and its not nice...

T_Base {
	doesNotUnderstand { |selector ...args|
		^\base
	}
}

T_Child : T_Base {
	doesNotUnderstandAbout { |selector, argumentsArray, keywordArgumentPairs|
		^\child
	}
}

T_Child2 : T_Base {
	doesNotUnderstandAbout { |selector, argumentsArray, keywordArgumentPairs|
		this.postln; 
		^\child
	}
}

T_Child().foo(1, 2) // returns \base

T_Child().foo(1, 2, bar: 3) // segfaults
T_Child2().foo(1, 2, bar: 3) // returns \child

I've been staring at this for a few hours so will have to take a break.

[telephon]

I have tried to find a way to have only one call back from the backend, but this doesn't work in the delegation logic as we would like it to.

We need this to work:

SubSubclass1().plink // -> \yes1 
SubSubclass2().plink // -> \yes2

In

Object {
	
	doesNotUnderstand { arg selector ... args;
		DoesNotUnderstandError(this, selector, args).throw;
	}

	doesNotUnderstandAbout { arg selector, args, keywordArgumentPairs;
		DoesNotUnderstandError(this, selector, args, keywordArgumentPairs).throw;
	}

}


Subclass : Object {
	
	
	doesNotUnderstandAbout { arg selector, args, keywordArgumentPairs;
		^\yes1	
        }
	
	
}

SubSubclass1 : Subclass {
	
	
	doesNotUnderstand { arg selector ... args;
		^super.doesNotUnderstand(selector, *args)
	}
	
}

SubSubclass2 : Subclass {
	
	
	doesNotUnderstand { arg selector ... args;
		^\yes2
	}
	
}