Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operators can only be defined one way #1371

Open
mkster opened this issue Jul 22, 2022 · 14 comments
Open

Operators can only be defined one way #1371

mkster opened this issue Jul 22, 2022 · 14 comments
Labels
enhancement New feature or request

Comments

@mkster
Copy link

mkster commented Jul 22, 2022

Describe the bug
Its seems like operators can only be defined one way? I want number * Vec3 to be Vec3. Am I missing something?

To Reproduce

    --- @class Vec3
    --- @operator mul(number): Vec3
    local Vec3 = {}

    local t1 = Vec3() * 2 ---Vec3
    local t2 = 2 * Vec3() --Integer!
@sumneko
Copy link
Collaborator

sumneko commented Jul 22, 2022

Dose not support class as the second parameter for now.

@sumneko sumneko added the enhancement New feature or request label Jul 22, 2022
@sewbacca
Copy link
Contributor

I would suggest that bi-operators accept two arguments, instead of one. One argument assumes that the first one allways will be the class itself, which is not the case A bi-operator function can only assume, that one of the arguments is of the type of the class, it is defined in.
In addition to that, the implied first parameter, is inconsistent. Check out this example:

---@class namespace.ClassName
---@operator unm(): namespace.ClassName

I would assume, that since add and mul, etc. assume that the first argument is of type namespace.ClassName, the __unm operator does assume it too, but the warning is confusing. However when I use the unary minus operator, it correctly assigns the type:

---@type namespace.ClassName
local b
local a = -b -- a is of type namespace.ClassName

What I would like to see, is explicit argument descriptors, for enabling different overloads:
Maybe the first parameter could be implied, if it is omitted. However one argument type allways has to be in the class it is defined in:

---@class A # operator add returns lastly added type
---@operator add(A): A
---@operator add(number): number
---@operator add(number, A): A
---@operator unm(A): A
---@operator unm(): A

Is there any reason @sumneko, why operator overloads cannot be implied from functions, defined in the class?

---@class A
local A = {}

---@generic T
---@param b T
---@return T
function A.__add(a, b)
    return b
end

@sumneko
Copy link
Collaborator

sumneko commented Jul 23, 2022

The current standard comes from #599 .
I can supports these at the same time:

---@class A
---@operator add: A --> eqauls to `add(A, any): A`
---@operator add(): A --> eqauls to `add(A, any): A`
---@operator add(number): A --> eqauls to `add(A, number): A`
---@operator add(number, A): A --> used for `1 + A`, but it may make people interested if it can be `add(number, B): A` in `class A`

Is there any reason @sumneko, why operator overloads cannot be implied from functions, defined in the class?

Strictly speaking, if I do not analyze the setmetatable, I cannot assume that the method of __add is related to additional operations.

@mkster
Copy link
Author

mkster commented Jul 23, 2022

Yeah I think this is needed:

---@operator add(number, A): A

With something like this there can also be this case

---@class A
---@operator add(A, B): A 

---@class B
---@operator add(A, B): B

local res = A + B -- A or B?

In that case res should be A because in lua metamethods the left table is checked before the right table for the presence of an arithmetic metaevent.

@sewbacca
Copy link
Contributor

sewbacca commented Jul 23, 2022

---@operator add(number, A): A --> used for 1 + A, but it may make people interested if it can be add(number, B): A in `class

Then probably a warning, that this annotation can never be used (or will be ignored), should be thrown.

Strictly speaking, if I do not analyze the setmetatable, I cannot assume that the method of __add is related to additional operations.

Is it possible to anlayze setmetatable() without performance issues?

@sumneko
Copy link
Collaborator

sumneko commented Jul 23, 2022

Is it possible to anlayze setmetatable() without performance issues?

Yes, just need analyze it once, then I can cache the result.

Unless I assume that the semantics of Class:__add is to add additional operations to Class.

---@class A
local mt = {}
mt.__index = mt
mt.__add = function () end

local t = setmetatable({}, mt)

In this code, I can infer local t is class A by __index, and I can infer t has "additional operation", but I cannot infer class A has "additional operation"

@CelticMinstrel
Copy link

The current standard comes from #599 . I can supports these at the same time:

---@class A
---@operator add: A --> eqauls to `add(A, any): A`
---@operator add(): A --> eqauls to `add(A, any): A`
---@operator add(number): A --> eqauls to `add(A, number): A`
---@operator add(number, A): A --> used for `1 + A`, but it may make people interested if it can be `add(number, B): A` in `class A`

These meanings seem quite strange to me. The most common use-case for operators, I believe, would be defining them to be symmetric. Thus, I would expect something more along the following lines:

---@class A
---@operator add:A --> add(A, A):A
---@operator add():A --> add(A, any):A and add(any, A):A
---@operator add(number):A --> add(A, number):A and add(number, A):A

Of course, there are times when it shouldn't be symmetric, so having a way to declare asymmetric operators (probably by specifying both parameters) is also important.

@tomlau10
Copy link
Contributor

tomlau10 commented Aug 3, 2024

Hi, I come from here #2777 (comment), and it contains a preliminary implementation idea for symmetric operator. Maybe someone would be interested to finish it and open a PR 😄 .

Btw I never thought of an asymmetric __add... what could be a possible use case for that 😕 ?


update: I see @SeanTheBuilder1 just implemented this symmetric operator logic in #2779 🎉

@sewbacca
Copy link
Contributor

sewbacca commented Aug 3, 2024

I do believe __add does happen less often but custom fractional types or var byte numbers or imaginary numbers should be able to use asymmetric operators to add together correctly.

These are still commutative though.
For multiplication we have scalars, vectors, matricies and quaternions. Here order is crucial for the output type.

If imaginaries handled both fractional and var byte numbers for addition but not the other way around, then an incorrect ordering could throw an error though.

@CelticMinstrel
Copy link

Btw I never thought of an asymmetric __add... what could be a possible use case for that 😕 ?

I don't know of a good use-case for an asymmetric __add specifically, but there are definitely cases where you can't just swap the arguments. For example, __mul on a matrix type would need different handling for A * B and B * A.

I don't know of any use-case where swapping the arguments has a completely different meaning and returns a different type though.

@tomlau10
Copy link
Contributor

tomlau10 commented Aug 4, 2024

on a matrix type would need different handling for A * B and B * A.

You're right, I understand that in matrix multiplication AB != BA as it is non commutative.

But when implementing it in programming wise, I guess there will only be a Matrix class and then each matrix object will have a row & column properties? i.e. both objects are in the same Matrix class. 😕

In this (object type) perspective, __mul on a matrix type seems to be symmetric. Because either AB or BA will still give a Matrix type. The row / column are just properties of the class object and unrelated to type inferencing?

@CelticMinstrel
Copy link

But when implementing it in programming wise, I guess there will only be a Matrix class and then each matrix object will have a row & column properties? i.e. both objects are in the same Matrix class. 😕

Well, it depends on the approach, but yes, if I were implementing a matrix in Lua, I'd probably do it as one type that has implements __mul and checks the dimensions of its arguments. It is probably possible to jump through some extra hoops and make something like a "matrix class factory" that takes a pair of dimensions and spits out a class representing a matrix of exactly those dimensions, but I don't think I'd do things that way in Lua.

As I said before, I can't think of an actual example in Lua where I'd want to overload an operator in a way that the arguments are of different types and can't be swapped.

But speaking of swapping, Lua doesn't actually swap the arguments (except for comparisons). Instead, it doesn't guarantee that the first argument is of the type owning the metatable. So if a custom overloaded operator can't be swapped, it just needs to raise an error if the first argument doesn't have the same metatable.

@sewbacca
Copy link
Contributor

sewbacca commented Aug 4, 2024

I would personally not implement matrix vector multiplication with the same class.

A vector has a lot of operatioms which are meaningless to a generic matrix.

While vector * matrix would give a vector, so all the usefull operations still apply, matrix * vector should give a 1x3 matrix, since it is similar to a vector but not the same thing.

@tomlau10
Copy link
Contributor

tomlau10 commented Aug 4, 2024

While vector * matrix would give a vector, so all the usefull operations still apply, matrix * vector should give a 1x3 matrix

In this case, both Vector and Matrix should have their own metamethods __mul. And by the definition from lua manual, the metatable of the left operand will be checked first, and then check the right operand. Therefore I think we can have the following annotation:

---@class Vector
---@operator mul(Vector): Vector
---@operator mul(Matrix): Vector
Vector = {}

---@class Matrix
---@operator mul(Matrix): Matrix
---@operator mul(Vector): Matrix
Matrix = {}

local a = Vector * Vector   --> Vector
local b = Vector * Matrix   --> Vector
local c = Matrix * Vector   --> Matrix
local d = Matrix * Matrix   --> Matrix

With or without the logic in #2779, which tries again with flipped argument when the first operator check failed, it will still work as expected. Because in both situations Vector * Matrix and Matrix * Vector, the left operand ALWAYS has a __mul metamethod that can cover the case 👀

            local node = vm.runOperator(binaryMap[op], source[1], source[2])
            -- `node` will always be non nil at here
            -- as both `Vector * Matrix` and `Matrix * Vector` is covered by `mul` operator overload annotation
            if not node then
                -- so this if case will never be executed
                node = vm.runOperator(binaryMap[op], source[2], source[1])
            end

Actually I am curious that for the 1 + A case, can we just add an operator overload for the number class?
But I tested it and seems not working 🤔

---@class Vector
---@operator mul(Vector): Vector
---@operator mul(number): Vector
Vector = {}

---@class number
---@operator mul(Vector): Vector

local a = Vector * 1   --> Vector
local b = 1 * Vector   --> unknown

Seems we cannot use operator overload for built-in classes 😅

I would say that with the help of #2779, symmetric operator will be the default behavior. And if we want to have asymmetric behavior we can ALWAYS add an operator overload rule in the other class (except built-in classes), like the Vector / Matrix example above. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants