Regression: Metatables no longer work

[DynamicCake]

On master, this test I wrote fails

pub fn metatable_test() {
  let lua = glua.new()
  let table = glua.table([])
  let proxy = glua.function(fn(lua, _args) { #(lua, [glua.string("three")]) })
  let metatable = glua.table([#(glua.string("__index"), proxy)])
  let assert Ok(#(lua, _)) =
    glua.ref_call_function_by_name(lua, ["setmetatable"], [table, metatable])
  let assert Ok(lua) = glua.set(lua, ["number_table"], table)
  let assert Ok(#(_lua, ["three"])) =
    glua.eval(
      lua,
      "return number_table.itshouldntmatterwhatiputhere",
      decode.string,
    )
}

with

let assert test/glua_test.gleam:23
 test: glua_test.metatable_test
 code: let assert Ok(#(_lua, ["three"])) =
    glua.eval(
      lua,
      "return number_table.itshouldntmatterwhatiputhere",
      decode.string,
    )
value: Error(UnexpectedResultType([DecodeError("String", "Nil", [])]))
 info: Pattern match failed, no pattern matched the value.

Previously on 1.1.3, this test succeeded

pub fn metatable_test() {
  let lua = glua.new()
  let #(lua, table) = glua.table(lua, #(glua.string, glua.string), [])
  let #(lua, proxy) =
    glua.function(lua, fn(lua, _args) {
      let #(lua, str) = glua.string(lua, "three")
      #(lua, [str])
    })
  let #(lua, metatable) =
    glua.table(lua, #(glua.string, fn(lua, val) { #(lua, val) }), [
      #("__index", proxy),
    ])
  let assert Ok(#(lua, _)) =
    glua.ref_call_function_by_name(lua, ["setmetatable"], [table, metatable])
  let assert Ok(lua) = glua.set(lua, ["number_table"], table)
  let assert Ok(#(_lua, ["three"])) =
    glua.eval(
      lua,
      "return number_table.itshouldntmatterwhatiputhere",
      decode.string,
    )
}

Please keep the simplified encoders since the 1.1.3 one was annoying to write.

[selenil]

Hi! Your test case can be rewritten as:

pub fn main() {
  let lua = glua.new()
  let table = glua.table([])
  let proxy = glua.function(fn(lua, _args) { #(lua, [glua.string("three")]) })
  let metatable = glua.table([#(glua.string("__index"), proxy)])
  let assert Ok(lua) = glua.set(lua, ["number_table"], table)
  let assert Ok(lua) = glua.set(lua, ["mt"], metatable)
  let assert Ok(#(lua, _)) =
    glua.ref_eval(lua, "setmetatable(number_table, mt)")
  let assert Ok(#(_lua, ["three"])) =
    glua.eval(
      lua,
      "return number_table.itshouldntmatterwhatiputhere",
      decode.string,
    )
}

and now it succeeds.

If you do not want to leave the metatable in the Lua environment, then you can create a helper function that takes it as argument, call the corresponding Lua code, and then return the updated Lua state.

I think we could advice folks in the documentation that the style of programming used in the original test case is not how luerl or this library are intended to be used. Both libraries are wrapping mutable state inside an immutable API and thus users should not expect that the value of an already defined variable will change when a function is called later on the code.

I am going to leave this issue open if someone encounter another problem with metatables.

[DynamicCake]

I disagree that this is a good way of doing things intended or not. Not having a way for tables to change state across glua.Luas only adds inconvenience to the user.

If a user wants to do the simple task of adding a metatable to a table, I think that it is unreasonable have user to

1. Set the table and metatable to be global and not accidentally override something
2. Call a function in stringified lua
3. glua.get_ref it
4. Clean up the globals and not mess up

If I was a new user, I wouldn't think this would be the correct way of doing things.

Both libraries are wrapping mutable state inside an immutable API and thus users should not expect that the value of an already defined variable will change when a function is called later on the code

I would argue that luerl is already wraps mutable state with an immutable API by using different luerlstate()s. We don't need to do anything else since luerl already does this. If we want to keep with the luerl project's intent, we should have a way of changing table state across glua.Luas. (It seems that keeping with luerl's intent is implied to be the goal)

In the deser branch, this isn't a problem since Values, formerly ValueRefs, are luerldata() and ask for a glua.Lua.

[selenil]

I agree that simple things like adding a metatable to a table is not intuitive at all for a new user and the way of do it right now leaves much to be desired. However, I think it's important to keep clear boundaries between what is immutable and what is not. One of the planned features I had for this library was a group of functions to manipulate and mutate a table in a more explicit way and those will come handy for cases like this.

It seems to me that issues like this hint at a deeper design problem. The library treats tables in the same way it treat other types, but tables are different since they require to be tracked in the Lua state. For now I will revert the table encoder to its previous form, which makes your test case succeed, and then explore ways of making common interactions with tables more ergonomic and less reliant in implicit mutability.

[DynamicCake]

One of the main problems with setting clear immutability boundaries by making tables immutable, is that it just isn't possible to make it consistent. Consider if a user does this

pub fn example_test() {
  let lua = glua.new()
  // This is the only way to create a ValueRef in glua
  let assert Ok(#(lua, [table])) = glua.ref_eval(lua, "tbl = {42} return tbl")
  let first = decode(table, lua)
  let lua = mystery_function(lua)
  // This will fail
  assert first == decode(table, lua)
}

fn mystery_function(lua) {
  let assert Ok(#(lua, _)) = glua.ref_eval(lua, "tbl[1] = 4")
  lua
}

/// Here for deomonstration purposes because as of now, glua has no way of doing anything with `ValueRef`s outside of function calls
/// This function is here to represet using a safe decoding function that glua will provide in the future
@external(erlang, "luerl", "decode")
fn decode(ref: glua.ValueRef, lua: glua.Lua) -> a

I have no idea how I would stop this from happening since table is still Tref(14) all the time.

In luerl, Tref(Int) is just a key to the luastate()'s map of tables. I think it is okay that when a map changes, the value associated with the key get is different. Even though this behavior is not exposed to the user, I think that the user could very easily be taught that Values change over different Luas just like how values change over different Dicts. This is a great compromise on Lua's mutability.

Although I don't think it is too important to make tables immutable and I don't see how it could be possible, I am still open to the idea and would I love to see a prototype.

[selenil]

Yes, you are right on this, treat tables as immutable if luerl does not treat them as such just creates problems and it would make the library very limited if we try do it. I reverted the table and userdata encoder as how they were before in #20, and I think the fact that both types need the Lua state to be encoded is enough to distinguish them from the other types. This way it will be clear to the user which types are immutable and which are mutable.