LuaTable behaves strangely (and uniquely) during type coercion

[Whoome]

NeoLua Version: 1.3.14

Setting up a simple example to demonstrate the odd, unexpected and (I think) undesirable behavior of LuaTable. In short LuaTable neither tries to coerce to type or just do nothing when used incorrectly as a parameter to a function. Rather it calls a default constructor (if one exists) and uses that.

I set up the following classes in C# to demonstrate

//Note that Foo is convertable to Bar
class Foo
{
    int m_value;

    public Foo()
    {
        m_value = 1071;
    }

    public Foo(int i)
    {
        m_value = i;
    }

    public int Value{ get{ return m_value; } }

    public override string ToString()
    {
        return string.Format("Foo({0})", Value);
    }
}

class Bar
{
    int m_value;
    public Bar(int i)
    {
        m_value = i;
    }

    public static implicit operator Foo(Bar b)
    {
        return new Foo(b.Value);
    }

    public int Value{ get{ return m_value; } }

    public override string ToString()
    {
        return string.Format("Bar({0})", Value);
    }
}

And the following functions exposed to the lua environment:

Foo MakeFoo(int i)
{
    return new Foo(i);
}

Bar MakeBar(int i)
{
    return new Bar(i);
}

void ProcessFoo(Foo f)
{
    PrintLine(string.Format("Processed {0}", f.ToString()));
}

void ProcessBar(Bar b)
{
    PrintLine(string.Format("Processed {0}", b.ToString()));
}

Example to reproduce:

>foo = MakeFoo(1)  --Setup for tests
>bar = MakeBar(2)
>ProcessFoo(foo)   --Process foo as foo
Processed Foo(1)
>ProcessBar(bar)   --Process bar as bar
Processed Bar(2)
>ProcessFoo(bar)  --Process bar as foo (calls conversion correctly)
Processed Foo(2)
>ProcessBar(foo)  --Process foo as bar: correctly fails to convert with good error
Exception: No conversion defined from Foo to Bar.
>ProcessFoo({})  --Process LuaTable as Foo:  GIVES SUPER WEIRD RESULT!  It called the empty constructor for Foo, and just went with it!
Processed Foo(1071)
>ProcessBar({})   --Process LuaTable as Bar: Fails weirdly too - why is it calling the default constructor?!?
Exception: Type 'Bar' does not have a default constructor
>ProcessBar("")  --Process a string as bar:  Behaves as expected gives an appropriate type coercion error
Exception: No coercion operator is defined between types 'System.String' and 'Bar'.

In general, I would expect LuaTable to act exactly as String or Foo or Bar do, if it is an inappropriate type to attempt type coercion, and if that is not possible, then I would expect an error.

[Whoome]

I can see where it is going wrong, but I don't have a feel for why it is written the way it is. In LuaTable.cs, around line 642:

class class LuaTableMetaObject
{
...
  public override DynamicMetaObject BindConvert(ConvertBinder binder)
  {
    // Automatic convert to a special type, only for classes and structure
    var typeInfo = binder.Type.GetTypeInfo();
    if (!typeInfo.IsPrimitive && // no primitiv
        !typeInfo.IsAssignableFrom(Value.GetType().GetTypeInfo()) && // not assignable by defaut
        binder.Type != typeof(LuaResult)) // no result
    {
        return new DynamicMetaObject(
            Lua.EnsureType(
                Expression.Call(Lua.EnsureType(Expression, typeof(LuaTable)), Lua.TableSetObjectMemberMethodInfo, Lua.EnsureType(Expression.New(binder.Type), typeof(object))),
                binder.ReturnType),
            GetLuaTableRestriction());
    }
    return base.BindConvert(binder);
  } // func BindConvert
...
}

Basically the if statement is getting hit causing it to NOT actually try and convert to non-primitive types. I extended the example slightly to show this and calling:

MakeFoo({})   --expects an int (a primitive type)
Exception: No conversion defined from LuaTable to Int32.  --Gives an expected coersion error (correct)

Now, clearly I could fix this up by just removing that entire IF statement (always just return base.BindConvert(binder)), since I can't see when it would actually be useful. But, it must be there for some reason that is just not obvious to me.

Edit OK. I see what this is trying to do, it allows me to try and convert to a data class by mapping data directly from the LuaTable to the record class. I can even see where that would sometimes be useful. But, it isn't the kind of behavior I would want by default or by accident and it seems so unintuitive - since the everything else is handled completely differently. Maybe some kind of switch so this behavior could be disabled?

[neolithos]

Correct analyzed. The idea was a quick class initialization.

local t : SomeClass = {
  M = 1,
  V = "test"
}

May this BindConvert-Code should be moved to the compiler? Or I will add more checks?

[Whoome]

Hmm, I wasn't aware of that syntax (It isn't standard lua, but it is pretty neat).

I think moving it to the compiler would be the way to go if possible, since then you would have parameter checks as well.

Right now if I do something like:

local t : SomeClass = {
     M = 1,
     v   = "test"    --Note the case incorrectness per the above example
}

M will get assigned and v silently goes nowhere. This feels like a difficult to find bug.

Also, moving it to the compiler will then let tables act just like everything else, so if I write a C# class like:

class InitFromTable
{
...  //Stuff

  public static operator InitFromTable(LuaTable tt)
  {
      //Conversion
  }
}
'''

Than any function that takes on of those, can just be called from a table.  And it is up to the implementer how to do the conversion.

[neolithos]

Implement a implicit converter should be the best solution. Because, it is clear to the implementer of the class.

I will try to move the assignment to the compiler. Get you write some test-cases? That would help to get it right.

[Whoome]

Sure... Maybe I'll extend my brilliant examples above :)

Are there test cases in the source base? I'll have to go look.

[neolithos]

There is one single test method TestConvert01() (LuaTable.cs:400).

• [x] table.set(obj, t, true|false), table.new(t, type, true|false)
• [x] I thing, we support explicit operators, which is working obj = { }.
• [x] Maybe type(){} or type()({}) for construction

Should be done in the compiler with this syntax.

• [x] Init type via cast(type, { })
• [ ] Init type via cast(type, table)
• [ ] Init type via local a : type = {}
• [x] var = { } is replaced with var{} or var({})

Have you more in ideas?

[Whoome]

Nothing off the top of my head... I'll think about it. Hopefully look at it a bit later this week.

[Whoome]

Wow... super fast. I'm still trying to unbury myself from work issues :)

[neolithos]

But not completely done. The final was a little bit early, after testing some in-house projects.

[Whoome]

My stupid test scenarios now run as I would expect them to. Sorry it too me so long to get back to things.

[neolithos]

But my not :). I hope I have time to look into it soon.