Object prototyping is NOT broken for higher oder functions – but WHY?

smoge · March 24, 2024, 10:54am

Undoubtedly, it is an improvement, and the documentation and comments are excellent.

The lesson is that adding redundancy increases complexity and failures, especially if they are undocumented.

jordan · March 24, 2024, 2:28pm

Just following up with a description what is changed in the interpreter for that PR…

When you call

\obj.asdf(1, 2, a: 3)

The following byte code is generated.

0   40          PushLiteral Symbol 'obj'
1   64          PushSpecialValue 1
2   65          PushSpecialValue 2
3   41          PushLiteral Symbol 'a'
4   2C 03       PushInt 3
6   B0          TailCallReturnFromFunction
7   0A 05 01 00 SendMsgX 'asdf'
11  F2          BlockReturn

In Line 7 0A 05 01 00:

0A means SendMsgX, which is send message with keyword arguments
05 is how many things are on the stack ('obj', 1, 2, 'a' & 3).
the next value 01 tells us how may keyword arguments there are. \obj.asdf(1, 2, a: 3, b: 4) produces 0A 07 02 00, for the 2 keyword args.
I don’t know what the final 00 is.

That PR just turns the stack from this:

obj, 1, 2, a, 3

into this

obj, [1, 2], [a, 3]

Then calls doesNotUnderstandWithKeys.

That would mean the interpreter would have to loop over all arguments to every sendMsg to see if the slot is a kind of KeywordArg, then switch behaviour, unpacking it, rather than switching in the parser. This could incur a performance cost as it would have to happen everywhere.

What happens if someone wrote this?

Synth(\x, KeywordArgs([\a: 1]), KeywordArgs([\b: 2]));

or

Synth(\x, KeywordArgs([\a: 1]), 2, KeywordArgs([\b: 2]));

Regarding expanding KeywordArgs automagically onto the stack…
I think switching on class when passing arguments will be confusing… but you could generalise this even more in a smalltalk way…

KeywordArgs {
   appendToArgumentStream{|stream| ... }
}

… which is kinda fun!

I don’t think the interpret should change. The code is old, fragile and hard to understand.

Instead, I think, sc should remove all existing ways of calling a function (all value*, perform* and the like…) and have only…

value, which would stay the same
valueWith{|args, kwargsPairs, lookupInCurrentEnvir| }
performWith{|selector, args, kwargsPairs, lookupInCurrentEnvir| }

and maybe functionPerformWith{|selector, args, kwards, lookupInCurrentEnvir| ... } for prototyping.

These would all be primitives.

This would incur a small performance cost over existing methods, but reduce the amount of C++ that needs to be maintained and, I think, it very explicit.

jamshark70 · March 24, 2024, 2:53pm

I think we could eliminate the “SendMsgX”.

There’s no way to process arguments without looping over them – when encountering a KeywordArgs, just dispatch to that part of the code. No need to overcomplicate.

Illegal, per language syntax definition. Keywords args must be in the last position, and there’s no reason to support multiple instances. (If the user does the above, 1. Runtime error for multiple KeywordArgs instances, which would be disallowed. 2. Warning because Synth *new has no a argument.)

hjh

julian · June 17, 2024, 7:24am

I have made an issue there:

github.com/supercollider/supercollider

misleading signature in performList

opened 07:11AM - 17 Jun 24 UTC

telephon

bug

This surfaced in #6339, thanks @JordanHendersonMusic See also https://scsynth.…org/t/object-prototyping-is-not-broken-for-higher-oder-functions-but-why/ And especially: https://scsynth.org/t/object-prototyping-is-not-broken-for-higher-oder-functions-but-why/9171/22?u=julian (thanks @jamshark70) It is not clear whether this bug (or inconsistent behaviour) should be fixed. This issue is to see what can be done about it. It also is the case for `Object::superPerformList` and `Object::functionPerform`. ## Environment * SuperCollider version: all versions ```supercollider performList { arg selector, arglist; _ObjectPerformList; ^this.primitiveFailed } ``` ## Steps to reproduce ```supercollider f = { |x, y, z| (x:x, y:y, z:z) }; // this is the correct and intended use: f.performList(\value, [2, 3, [1, 2]]); // ('x': 2, 'y': 3, 'z': [1, 2]) // when passing inlined arguments, it behaves "funny": it embeds an array iff it is the last argument. f.performList(\value, 2, 3, [1, 2], 5); // ('x': 2, 'y': 3, 'z': [1, 2]) f.performList(\value, 2, 3, [1, 2]); // ('x': 2, 'y': 3, 'z': 1) ``` ## Expected vs. actual behavior ```supercollider f = { |x, y, z| (x:x, y:y, z:z) }; // correct standard use f.performList(\value, [2, 3, [1, 2], 5]); // ('x': 2, 'y': 3, 'z': [1, 2]); // same as f.value(2, 3, [1, 2], 5); // ('x': 2, 'y': 3, 'z': [1, 2]); f.valueArray([2, 3, [1, 2], 5]); // ('x': 2, 'y': 3, 'z': [1, 2]); // actual f.performList(\value, 2, 3, [1, 2], 5); // ('x': 2, 'y': 3, 'z': [1, 2]) f.performList(\value, 2, 3, [1, 2]); // ('x': 2, 'y': 3, 'z': 1) // expected strictly judging from the interface (arg selector, arglist) f.performList(\value, 2, 3, [1, 2], 5); // ('x': 2, 'y': nil, 'z': nil) f.performList(\value, 2, 3, [1, 2]); // ('x': 2, 'y': nil, 'z': nil) // expected maybe with some leeway f.performList(\value, 2, 3, [1, 2], 5); // ('x': 2, 'y': 3, 'z': [1, 2]) f.performList(\value, 2, 3, [1, 2]); // ('x': 2, 'y': 3, 'z': [1, 2]) ```

jordan · June 27, 2024, 11:30am

Just coming back to this so all threads of conversation are in sync…

TLDR: the interpreter knows best, performList is variable, and keyword arguments are currently unfinished in their implementation.

There are two bugs with performList, which at first seem unconnected

1.

The cpp declaration of the primitive has variable arguments, but the sc code doesn’t.
When the code is parsed, an optimisation is made when it is just a primitive call or just relays to another method.
There is another bug here as it doesn’t copy nor check the variable argument status of the primitive, but assumes the programmer has written the signature correctly in sc — this is why this bug has lasted for 15+ years.
While the actual Method object will tell you that it doesn’t have variable arguments, the interpreter secretly knows that it does and forces the behaviour.

2.

f = {|a, b, c, d| [a, b, c, d] };
f.(1, *[2, 3], d: 10); 
// Warning keyword 'd' not found in call to Object:performList.

When you use the *[...] syntax, the parser wraps it in a call to performList rather than value. This means the user can change what message is sent by the argument — this is very unusual behaviour, perhaps even unique in supercollider?

Here is an example of what the parser and interpreter actually do.

f.(1, 2, d: 10) -> f.value(1, 2, 3, d: 10);

f.(1, *[2, 3], d: 10) -> f.performList(\value, 1, [2, 3], d: 10)

Two things can be seen from this.

performList must have a variable argument because the interpreter needs to pass arbitrarily many arguments, where the last argument should be unpacked onto the stack. Actually, it already does have a variable argument as far as the interpreter is concerned, the reflection in sc just can’t see the whole truth, and changing this to a variable argument has zero effect on the rest of the code base. What is more, many quarks call performList by constructing a single array rather than passing the arguments directly, which is slow and unnecessary. To remove the variable argument would mean changing a fundamental way the interpreter has worked for 15+ years — IMO, this is a very bad idea.
If we don’t want the user to know that using the *[...] relays through performList rather than going straight to value, we must ensure that they are homomorphic. If this isn’t the case, every time the user writes *[...] they must check the implementation to see if it does relay to value or does something else. IMO, this would be incredibly confusing. Imagine this…

f.(1, 2, 3) \\ 10
f.(1, *[2, 3]) \\ 20

Keyword arguments are broken and there needs to be a way to pass keyword arguments along
SCLang, ClassLib: Add *kwargs by JordanHendersonMusic · Pull Request #6339 · supercollider/supercollider · GitHub

What does ObjectPerformList do?

There’s been some confusion over this, but its actually very simple.

It takes a selector, and any number of arguments, if the last argument is an array, it unpacks it onto the stack next to the other arguments.

Here are some examples showing how the stack changes.

Existing behaviour using f.(1, *[2, 3]) or f.performList(\value, 1, [2, 3])

stack in: 
    numargs = 4
    layout = f, \value, 1, [2, 3]

stack after unpacking:
    numargs = 5
    layout = f, \value, 1, 2, 3

stack after removing selector and just before calling ObjectPerform (on value):
    numargs = 4
    layout = f, 1, 2, 3

Existing behaviour using f.(1, *[2, 3], d: 10) or f.performList(\value, 1, [2, 3], d: 10)

stack before keyword lookup:
    numargs = 6, numKwArgs = 1
    layout = f, \value, 1, [2, 3], \d, 10

prints: keyword d not found in ObjectPerformList 
stack after applying keywords:
    numargs = 4
    layout = f, \value, 1, [2, 3]

... same as before

Existing behaviour using f.(1, *[2, 3], selector: 10) or f.performList(\value, 1, [2, 3], selector: 10)

stack before keyword lookup:
    numargs = 6, numKwArgs = 1
    layout = f, \value, 1, [2, 3], \selector, 10

stack after applying keywords:
    numargs = 4
    layout = f, 10, 1, [2, 3]
                ^^^ uh-oh! attempting to call f.10(1, 2, 3)

Can we have proper wrapping methods?

Not with the current behaviour due to keywords being unfinished.
With the keyword pr this is simply written as.

foo { |...args, kwargs|
   ...
}

For performList to be a proper wrapper, and homomorphic to value, it is necessary to drop that selector argument, otherwise that keyword would be swallowed by performList. I’ve written in more detail on this and why it is a trivial breaking change.

You can’t, however, extend a wrapper function with more arguments later, there is only one perfect wrapping signature. This makes sense, because it would imply a third kind of argument, not a normal argument, not a keyword arguments, but something else.

Assuming the keyword arg pr (which changes performList to be a proper wrapper), the stack does this, for
f.(1, *[2, 3], selector: 10)

stack before keyword lookup:
    numargs = 6, numKwArgs = 1
    layout = f, \value, 1, [2, 3], \selector, 10

(There are no keywords in performList so they are all left)

stack after unpacking:
    numargs = 7, numKwArgs = 1
    layout = f, \value, 1, 2, 3, \selector, 10

stack after removing selector
    numargs = 6, numKwArgs = 1
    layout = f, 1, 2, 3, \selector, 10

Now it calls ObjectPerformWithKeys using the \value selector, there the keywords are handled.

Regarding architecture.

One thing that maybe hasn’t been mentioned is how people’s conception of how something works (particularly something 15+ years old) can often drift from how it actually works. This is particularly true for these primitives which are called by users and the interpreter, sometimes in different ways.

sorry for the big post.

jamshark70 · June 28, 2024, 8:34am

Are variable-args reflected in the Method object? Not in the argNames array, AFAICS.

Hm, but…

{ a[1] }.def.dumpByteCodes
BYTECODES: (6)
  0   12       PushInstVar 'a'
  1   64       PushSpecialValue 1
  2   B0       TailCallReturnFromFunction
  3   C2 02    SendSpecialMsg 'at'
  5   F2       BlockReturn

{ a[1..3] }.def.dumpByteCodes
BYTECODES: (9)
  0   12       PushInstVar 'a'
  1   64       PushSpecialValue 1
  2   6E       PushSpecialValue nil
  3   2C 03    PushInt 3
  5   B0       TailCallReturnFromFunction
  6   A4 00    SendMsg 'copySeries'
  8   F2       BlockReturn

… which is stretching the case a little bit, because f.value(...) specifies a method selector explicitly while a[...] is a shortcut. But we usually assume a[] is ‘at’, but it isn’t always ‘at’, and the format of the argument list makes the difference.

While I understand your objection here, it doesn’t very very deeply disturb me…? It’s something that a future SC4 could do better, sure.

hjh

jordan · June 28, 2024, 9:04am

I didn’t think of this case, thanks!

However, a[1] and a[1..3] both describe different user intentions, the first says, ‘get me a value’, the second, ‘get me a slice’. Having different behaviour here is expected.

Whereas f.(1, 2, 3) and f.(1, *[2, 3]) are both intended to ‘evaluate’ the function, with the latter performing an extra step on the array.

Also (1..3) is valid syntax that produces an array, and having the ‘at’ method change based on what type is passed in is quite familiar. Whereas * isn’t valid syntax anywhere else. In fact, because the little star is applied to the array and not the function, I’d expect the following to work…

var arg1, arg2, rest;
#[arg1, arg2... rest] = *someArray

… and the fact is doesn’t is a little surprising, but not a big deal and perhaps a different discussion.

My point here is merely, having more than one definition of ‘evaluate’ is very confusing, and where performList performs extra stuff, it should be as transparent as possible, and not change anything unintended — it should only unpack the final arg if its an array. The kwargs pr makes it completely transparent.

As an aside, I’m also considering (if I get time and can figure it out) adding a way of passing on kwargs using syntax, it would look like:

args = [2, 3];
kwargs = [\foo, 10, \bar, 20];

f.(1, *args, **kwargs)

// which turns into

f.(1, 2, 3, foo: 10, bar: 20)

I’m not objecting to *[] syntax.
I’m objecting to performList not functioning as a perfect transparent wrapper (which I’ve fixed).
EDIT: This is also the main cause of the confusion that generated this thread.

Yup, its an undocumented method called varArgs. Its in FunctionDef though.