add rdiv and friends (only radd is already implemented)

[benmkw]

like defined in https://docs.python.org/3.11/reference/datamodel.html#emulating-numeric-types

this is helpful for emulating/ creating apis like the python ones of z3 or sympy, using starlark as the scripting engine instead of python, writing the core logic in rust

[stepancheg]

Although Starlark is designed mainly as a configuration language and not as an all-purpose scripting language, adding operations like rdiv would make it more consistent.

[ndmitchell]

Part of the problem is that radd has a fair amount of complexity and cost, since when you see a + b you have to call radd to see if that is interested, and then make another virtual call to add after. The only reason for having radd is because its required for select in Buck, which really annoys me. @stepancheg - do you think we can do things like rdiv without the cost? Or maybe since + is by far the most common operator, everything else is perhaps irrelevant to performance anyway?

[stepancheg]

you have to call radd to see if that is interested, and then make another virtual call to add after

We actually changed that after adding special cases for strings and numbers, so the overhead of looking up for add first then radd, and having Options is not critical.

087f79a

(I don't see a benchmark on the diff, but I think I checked it is fine).

To answer your question, vast majority of binops are +, and the rest are far less critical, and again we can do special cases for common types.

[benmkw]

thanks for looking into this so quickly, just a note, I recently read a bit about performance of dunder methods in python and opened an issue there faster-cpython/ideas#555 so that might be interesting for starlark as well

[benmkw]

To answer your question, vast majority of binops are +, and the rest are far less critical, and again we can do special cases for common types.

Does this mean that this issue could be implemented? I'm not yet sure if its basically just a matter of copying some stuff in https://github.com/facebookexperimental/starlark-rust/blob/5febddfb306956ca11c88e2b75ac7acf631b1bac/starlark/src/values/traits.rs#L568-L589 and probably https://github.com/facebookexperimental/starlark-rust/blob/5febddfb306956ca11c88e2b75ac7acf631b1bac/starlark/src/values/layout/vtable.rs#L422-L429 or if there are more invasive changes needed?

[stepancheg]

its basically just a matter of copying some stuff

Yes.

[stepancheg]

I recently read a bit about performance of dunder methods in python and opened an issue there ... so that might be interesting for starlark as well

No. Starlark does not allow operator overloading in non-native code, so the problem does not exist in starlark-rust. Operator call is basically a simple virtual call (or two, in case of add/radd).

Also we use "slots" for builtin methods (like list.append).

[benmkw]

its basically just a matter of copying some stuff

Yes.

just for reference if someone comes around this: dc68400 which is probably a blueprint for the remaining ops

[cjhopman]

careful with that as a blueprint. it looks like there's a bug in that it doesn't actually call rmul in the reverse case (it just calls mul). This bug appears to still exist in head (cc @stepancheg)

[stepancheg]

@cjhopman fix in D49195881.