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

Support metatables for GoStruct #74

Open
andreysm opened this issue Feb 7, 2020 · 4 comments
Open

Support metatables for GoStruct #74

andreysm opened this issue Feb 7, 2020 · 4 comments

Comments

@andreysm
Copy link

andreysm commented Feb 7, 2020

I'm trying to set a metatable to a GoStruct object to be able calling methods in the manner like obj:Foo().

Currently, testudata() doesn't allow me to do this, requiring the object metatable to be MT_GOINTERFACE or MT_GOFUNCTION.

I understand the idea of this check, and suggest extending it in the following manner:

if (!lua_rawequal(L, -1, -2))  /* not the same? */

    // Check the second-level metatable
    if (lua_getmetatable(L, -2)) {
        if (lua_rawequal(L, -2, -1)) {  /* equals! */
            lua_pop(L, 3);  /* cleanup */
            return p;
        }
        lua_pop(L, 1);
    }
    // end of new code

    p = NULL;
}
lua_pop(L, 2);  /* remove both metatables */
return p;

In addition, the setmetatable() call should be modified (as following), or a new function (e.g. SetGoStructMetaTable) may be introduced if you find this way more appropriate.

void clua_setmetatable(lua_State* L, unsigned int index)
{
    if (clua_isgostruct(index)) {
        // Set MT_GOINTERFACE as a second-level metatable for the user metatable
        luaL_getmetatable(L, MT_GOINTERFACE);
        lua_setmetatable(L,-2);
    }

    lua_setmetatable(L, index);
    return true;
}
@aarzilli
Copy link
Owner

Sounds reasonable. Care to make a PR?

@andreysm
Copy link
Author

andreysm commented Feb 16, 2020

My original idea was quite incorrect: metatable on metatable won't work as I expected. But I managed to implement it another way.

Pull request: #78

I added a function SetMetaTableForGoStruct(), implemented via clua_gostructmetatable(), it modifies user-provided metatable by setting __gc, __index and __newindex fields, just as in MT_GOINTERFACE.
If the metatable already had __index or __newindex fields set by user, these values are stored as upvalues (i.e. in a closure) and then used later in interface_index_callback() or interface_newindex_callback(). With corresponding changes in golua_interface_index_callback() and golua_interface_newindex_callback(), these values gets "wrapped" with the callbacks and eventually work as expected by user.

The golua_interface_index_callback() was slightly modified to return '0' when there is no such key in the Go struct. When this occurs, interface_index_callback() checks for a user-provided __index value in the metatable (was previously stored as an upvalue) and then mimics how metatables works in lua.
This function still raises an error when there is no such field in the Go struct and there is no __index value in the metatable provided by user, so the behaviour should be as previous, without change.

The testudata() function additionally checks if the metatable.__gc field equals to the gchook_wrapper to detect for a user-provided metatable (modified by clua_gostructmetatable()). I believe this is an acceptabe solution and shouldn't cause any problem.

Tested in TestGoStructMetatable(), seems to work. 😃

@andreysm
Copy link
Author

Sorry, there is a bug with this solution.
I just made testudata(..., MT_GOFUNCTION) return true for MT_GOINTERFACE and vice-versa.
I'll think on how to fix it properly.

@andreysm
Copy link
Author

Well, I fixed it.
Pull request: #79

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

No branches or pull requests

2 participants