SCLang, ClassLib: Add keyword argument syntax ... kwargs (supercollid…

…er#6339)

SCLang and ClassLib: implement ... kwargs

This commit makes many changes, but because of how the interpreter bootstraps itself, it isn't possible to pull these out into smaller atomic steps that will each pass the CI.

Implement a new syntax for passing keyword arguments.
This commit should not change behaviour.

Update parse node code so it is compiler correctly.

Refactor out putting the arguments onto the stack for a method call so the logic for keyword collisions and other issues is always the same. Further, make this function take an optional environment to look values up in. This is called `prepareArgsForExecute`.

Use this new function to rewrite `executeMethod`. Since the logic for all method calls goes through only one function, `executeMethodWithKeys` can be removed. Do the same for `blockValue`, `blockValueWithKeys`, `blockValueEnvir` and `blockValueEnvirWithKeys`, these other methods can't be removed because they are primitive functions and a stable pointer to them is needed.

Similarly, `doesNotUnderstand` is unified with `doesNotUnderstandWithKeys`, refactored, and made to pass the new keyword argument array correctly.

The logic from `sendMessage`, `sendMessageWithKeys`, `superSendMessage` and `superSendMessageWithKeys` is extracted into one function and deletes the `*WithKeys` functions.

An additional primitive, `objectPerformArgsAndKwArgs` is created that allows you to pass an keyword argument array as this is impossible to do otherwise.

The sc method `performList`'s signature is changed to fix a longstanding bug with keyword arguments and `*[]` array unpacking of arguments.
The cpp function objectperformListWithKeys is also created to fix this.

Now in the class library Object is given a new method, `performWithArgsAndKwArgs` allowing to pass an array to the keyword arguments array.

DoesNotUnderstand is updated to allow keywords to be reported.

Object's `doesNotUnderstand` method is updated to accept keyword arguments and passes these to the error correctly.

TestKwargs.sc is created that tests all this behaviour.

Further, the documentation is updated. This required a new primitive to correctly report how many variable arguments there are.

* ClassLib: fix functionPerformList to use _ObjectPerformList primitive to conform to its namesake.
* ClassLib: Add test to TestKwargs.sc to check functionPerformList is working correctly.
* SCLang: unite the primitives superPerformList with performList, adding support for keyword arguments
* SCLang: Remove unused class, update headers in touched files, insert static_casts on touched code, new line at end of file.
* SCLang: Comment PyrMessage.cpp, remove unnecessary variables, generally improve readability. Also comment PyrKernel.h as one of the slots is used oddly and not mentioned.
* SCLang: Add comments, simplify code in performListTemplate.
* SCLang: Improve error message in performList variants
* ClassLib: Add test of performArgs
* SCLang: Add HOT to block frame creation in blockValueWithKeys. This prefers functions with normal arguments, matching keywords and no variable arguments.

---------

authored-by: JordanHendersonMusic <[email protected]>, 
co-authored by Julian Rohrhuber and  Dennis Scheiba

HelpSource/Classes/Object.schelp

@@ -626,6 +626,17 @@ subsection::Error Support
 
 Object implements a number of methods which throw instances of Error. A number of methods (e.g. doesNotUnderstand) are 'private' and do not normally need to be called directly in user code. Others, such as those documented below can be useful for purposes such as object oriented design (e.g. to define an abstract interface which will be implemented in subclasses) and deprecation of methods. The reserved keyword thisMethod can be used to refer to the enclosing method. See also Method and Function (for exception handling).
 
+method::doesNotUnderstand
+
+argument::selector
+The method that was called.
+
+argument::... args
+An link::Classes/Array:: of arguments.
+
+argument::kwargs
+An link::Classes/Array:: of keyword argument pairs.
+
 method::throw
 
 Throws the receiver as an Exception, which may or may not be caught and handled by any enclosing Function.

SCClassLibrary/Common/Collections/Dictionary.sc

@@ -539,30 +539,28 @@ IdentityDictionary : Dictionary {
 		if(parent.notNil) { stream << "\n.parent_(" <<< parent << ")" };
 	}
 
-	doesNotUnderstand { arg selector ... args;
-		var func;
-		if (know) {
-
-			func = this[selector];
-			if (func.notNil) {
-				^func.functionPerformList(\value, this, args);
-			};
-
-			if (selector.isSetter) {
-				selector = selector.asGetter;
-				if(this.respondsTo(selector)) {
-					warn(selector.asCompileString
-						+ "exists as a method name, so you can't use it as a pseudo-method.")
-				};
-				^this[selector] = args[0];
-			};
-			func = this[\forward];
-			if (func.notNil) {
-				^func.functionPerformList(\value, this, selector, args);
-			};
-			^nil
+	doesNotUnderstand { |selector... args, kwargs|
+		if (know.not) {
+            ^super.performArgs(\doesNotUnderstand, args, kwargs)
 		};
-		^this.superPerformList(\doesNotUnderstand, selector, args);
+        this[selector] !? { |func|
+            ^func.performArgs(\functionPerformList, [\value, this] ++ args, kwargs);
+        };
+
+        if (selector.isSetter) {
+            selector = selector.asGetter;
+            if(this.respondsTo(selector)) {
+                warn(selector.asCompileString
+                    + "exists as a method name, so you can't use it as a pseudo-method.")
+            };
+            ^this[selector] = args[0];
+        };
+
+        this[\forward] !? { |func|
+            ^func.performArgs(\functionPerformList, [\value, this, selector] ++ args, kwargs);
+        };
+
+        ^nil
 	}
 
 		// Quant support.

SCClassLibrary/Common/Collections/Scale.sc

@@ -18,9 +18,9 @@ Scale {
 		^all.at(key)
 	}
 
-	*doesNotUnderstand { |selector, args|
-		var scale = this.newFromKey(selector, args).deepCopy;
-		^scale ?? { super.doesNotUnderstand(selector, args) };
+	*doesNotUnderstand { |selector... args, kwargs|
+		var scale = this.performArgs(\newFromKey, [selector] ++ args, kwargs).deepCopy;
+		^scale ?? { super.performArgs(\doesNotUnderstand, [selector] ++ args, kwargs) }
 	}
 
 	*newFromKey { |key, tuning|
@@ -184,9 +184,9 @@ Tuning {
 		^all.at(key)
 	}
 
-	*doesNotUnderstand { |selector, args|
-		var tuning = this.newFromKey(selector, args).deepCopy;
-		^tuning ?? { super.doesNotUnderstand(selector, args) }
+	*doesNotUnderstand { |selector... args, kwargs|
+		var tuning = this.performArgs(\newFromKey, [selector] ++ args, kwargs).deepCopy;
+		^tuning ?? { super.performArgs(\doesNotUnderstand, [selector] ++ args, kwargs) }
 	}
 
 	*newFromKey { | key |

SCClassLibrary/Common/Core/Error.sc

@@ -147,9 +147,13 @@ ShouldNotImplementError : MethodError {
 }
 
 DoesNotUnderstandError : MethodError {
-	var <>selector, <>args, <suggestedCorrection, suggestion = "";
-	*new { arg receiver, selector, args;
-		^super.new(nil, receiver).selector_(selector).args_(args).init
+	var <>selector, <>args, <>keywordArgumentPairs, <suggestedCorrection, suggestion = "";
+	*new { arg receiver, selector, args, keywordArgumentPairs;
+		^super.new(nil, receiver)
+		.selector_(selector)
+		.args_(args)
+		.keywordArgumentPairs_(keywordArgumentPairs)
+		.init
 	}
 
 	init {
@@ -180,6 +184,10 @@ DoesNotUnderstandError : MethodError {
 		receiver.dump;
 		"ARGS:\n".post;
 		args.dumpAll;
+		keywordArgumentPairs !? {|kw|
+			"KEYWORD ARGUMENTS:".postln;
+			kw.dumpAll;
+		};
 		this.errorPathString.post;
 		if(protectedBacktrace.notNil, { this.postProtectedBacktrace });
 		this.dumpBackTrace;

SCClassLibrary/Common/Core/Function.sc

@@ -53,9 +53,9 @@ Function : AbstractFunction {
 		// unsupplied argument names are looked up in the currentEnvironment
 		^this.primitiveFailed
 	}
-	functionPerformList { arg selector, arglist;
-		_ObjectPerformList;
-		^this.primitiveFailed
+	functionPerformList { |... args, kwargs|
+        _ObjectPerformList;
+        this.primitiveFailed
 	}
 
 	valueWithEnvir { arg envir;

SCClassLibrary/Common/Core/Kernel.sc

@@ -440,6 +440,16 @@ FunctionDef {
 		_FunDef_VarArgs
 		^this.primitiveFailed
 	}
+	varArgsValue {
+		_FunDef_VarArgsValue
+		^this.primitiveFailed
+	}
+	hasVarArgs {
+		^this.varArgsValue > 0
+	}
+	hasKwArgs {
+		^this.varArgsValue > 1
+	}
 	shallowCopy { ^this }
 
 	asFunction {

SCClassLibrary/Common/Core/Object.sc

@@ -1,4 +1,4 @@
-Object  {
+Object {
 	classvar <dependantsDictionary, currentEnvironment, topEnvironment, <uniqueMethods;
 
 	const nl = "\n";
@@ -75,20 +75,24 @@ Object  {
 	isMemberOf { arg aClass; _ObjectIsMemberOf; ^this.primitiveFailed }
 	respondsTo { arg aSymbol; _ObjectRespondsTo; ^this.primitiveFailed }
 
-	performMsg { arg msg;
+    // args and kwargs should be arrays here, not variable arguments!
+	performArgs { |selector, args, kwargs|
+		_ObjectPerformArgs;
+		^this.primitiveFailed
+	}
+	performMsg { |msg|
 		_ObjectPerformMsg;
 		^this.primitiveFailed
 	}
-
 	perform { arg selector ... args;
 		_ObjectPerform;
 		^this.primitiveFailed
 	}
-	performList { arg selector, arglist;
+	performList { | ...args, kwargs|
 		_ObjectPerformList;
 		^this.primitiveFailed
 	}
-	functionPerformList {
+	functionPerformList { | ...args, kwargs|
 		// perform only if Function. see Function-functionPerformList
 		^this
 	}
@@ -97,20 +101,21 @@ Object  {
 	// \perform would be looked up in the superclass, not the selector you are interested in.
 	// Hence these methods, which look up the selector in the superclass.
 	// These methods must be called with this as the receiver.
-	superPerform { arg selector ... args;
+	superPerform { | ... args, kwargs|
 		_SuperPerform;
 		^this.primitiveFailed
 	}
-	superPerformList { arg selector, arglist;
+	superPerformList { | ...args, kwargs|
 		_SuperPerformList;
 		^this.primitiveFailed
 	}
 
-	tryPerform { arg selector ... args;
-		^if(this.respondsTo(selector),{
-			this.performList(selector,args)
+	tryPerform { |  ... args, kwargs|
+		^if(this.respondsTo(args[0]), {
+			this.performArgs(args[0],  args[1..], kwargs)
 		})
 	}
+
 	multiChannelPerform { arg selector ... args;
 		^flop([this, selector] ++ args).collect { |item|
 			performList(item[0], item[1], item[2..])
@@ -130,7 +135,7 @@ Object  {
 			val !? { args[i] = val };
 		};
 
-		^this.performList(selector, args)
+		^this.performArgs(selector, args);
 	}
 
 	performKeyValuePairs { |selector, pairs|
@@ -338,8 +343,8 @@ Object  {
 	subclassResponsibility { arg method;
 		SubclassResponsibilityError(this, method, this.class).throw;
 	}
-	doesNotUnderstand { arg selector ... args;
-		DoesNotUnderstandError(this, selector, args).throw;
+	doesNotUnderstand { |selector ...args, kwargs|
+		DoesNotUnderstandError(this, selector, args, kwargs).throw;
 	}
 	shouldNotImplement { arg method;
 		ShouldNotImplementError(this, method, this.class).throw;

SCClassLibrary/SCDoc/SCDoc.sc

@@ -384,7 +384,7 @@ SCDocNode {
 SCDoc {
 
 	// Increment this whenever we make a change to the SCDoc system so that all help-files should be processed again
-	classvar version = 73;
+	classvar version = 74;
 
 	classvar <helpTargetDir;
 	classvar <helpTargetUrl;

SCClassLibrary/SCDoc/SCDocRenderer.sc

@@ -174,11 +174,18 @@ SCDocHTMLRenderer {
 		var res = "";
 		var value;
 		var l = m.argNames;
-		var last = l.size-1;
+		var last = l.size - m.varArgsValue;
 		l.do {|a,i|
-			if (i>0) { //skip 'this' (first arg)
-				if(i==last and: {m.varArgs}) {
-					res = res ++ " <span class='argstr'>" ++ "... " ++ a;
+			if (i > 0) { //skip 'this' (first arg)
+				if(i >= last and: {m.hasVarArgs}) {
+					if(i == last){
+						if(i != 0){
+							res = res ++ " "
+						};
+						res = res ++ "<span class='argstr'>" ++ "... " ++ a
+					} {
+						res = res ++ ", <span class='argstr'>" ++ "... " ++ a
+					}
 				} {
 					if (i>1) { res = res ++ ", " };
 					res = res ++ "<span class='argstr'>" ++ a;
@@ -749,7 +756,11 @@ SCDocHTMLRenderer {
 				stream << "<tr><td class='argumentname'>";
 				if(node.text.isNil) {
 					currentMethod !? {
-						if(currentMethod.varArgs and: {currArg==(currentMethod.argNames.size-1)}) {
+						if(currentMethod.hasVarArgs
+							and: {
+								currArg == (currentMethod.argNames.size - (currentMethod.hasKwArgs.if({2}, {1})))
+							}
+						) {
 							stream << "... ";
 						};
 						stream << if(currArg < currentMethod.argNames.size) {
@@ -766,11 +777,12 @@ SCDocHTMLRenderer {
 					stream << if(currentMethod.isNil or: {currArg < currentMethod.argNames.size}) {
 						currentMethod !? {
 							f = currentMethod.argNames[currArg].asString;
-							if(
-								(z = if(currentMethod.varArgs and: {currArg==(currentMethod.argNames.size-1)})
-										{"... "++f} {f}
-								) != node.text;
-							) {
+							z = if(currentMethod.hasVarArgs and: {currArg == ( currentMethod.argNames.size - currentMethod.varArgsValue)}) {
+								"... " ++ f
+							} {
+								f
+							};
+							if(z != node.text) {
 								"SCDoc: In %\n"
 								"  Method %% has arg named '%', but doc has 'argument:: %'.".format(
 									currDoc.fullPath,

lang/LangPrimSource/PyrListPrim.h

@@ -25,6 +25,7 @@ void initArrayPrimitives();
 
 int prArrayMultiChanExpand(VMGlobals* g, int numArgsPushed);
 
+bool identDict_lookupNonNil(PyrObject* dict, PyrSlot* key, int hash, PyrSlot* result);
 int arrayAtIdentityHash(PyrObject* array, PyrSlot* key);
 int arrayAtIdentityHashInPairs(PyrObject* array, PyrSlot* key);
 int arrayAtIdentityHashInPairsWithHash(PyrObject* array, PyrSlot* key, int hash);

lang/LangPrimSource/PyrMathPrim.cpp

@@ -187,7 +187,7 @@ template <typename Functor> inline int prOpNum(VMGlobals* g, int numArgsPushed)
         return errFailed; // arguments remain on the stack
 
     msg = gSpecialBinarySelectors[g->primitiveIndex];
-    sendMessage(g, msg, 2);
+    sendMessage(g, msg, 2, 0);
     return errNone;
 }
 
@@ -233,7 +233,7 @@ template <typename Functor> inline int prOpInt(VMGlobals* g, int numArgsPushed)
         return errFailed; // arguments remain on the stack
 
     msg = gSpecialBinarySelectors[g->primitiveIndex];
-    sendMessage(g, msg, 2);
+    sendMessage(g, msg, 2, 0);
     return errNone;
 }
 
@@ -279,7 +279,7 @@ template <typename Functor> inline int prOpFloat(VMGlobals* g, int numArgsPushed
         return errFailed; // arguments remain on the stack
 
     msg = gSpecialBinarySelectors[g->primitiveIndex];
-    sendMessage(g, msg, 2);
+    sendMessage(g, msg, 2, 0);
     return errNone;
 }