stack overflow due to recursion in instance:base()

[Tieske]

function test:reset()
  assert(test:class_of(self), "expected self to be a test class")
  self:base("reset")          -- call ancestor
  self.flushed = nil          -- if thruthy, then output was already written
end

This seems to work as a workaround;

test.refcount = 0
function test:reset()
  test.refcount= test.refcount +1
  if test.refcount==1 then
    assert(test:class_of(self), "expected self to be a test class")
    self:base("reset")          -- call ancestor
    self.flushed = nil          -- if thruthy, then output was already written
  end  
  test.refcount= 0
end

it seems the call self:base("reset") is the culprit causing the recursion.

Any ideas what might be the problem?

[Tieske]

here's a minimal example showing it:

-- animal.lua

class = require 'pl.class'

--------Animal----------
local Animal = class()

function Animal:_init(name)
  self:reset()
  self.name = name
end

function Animal:reset()
  self.legs = nil
end

---------Cat------------
local Cat = class(Animal)

function Cat:_init(name,breed)
   self:super(name)  -- must init base!
   self.breed = breed
end

function Cat:reset()
  self:base("reset")
  self.legs = 4
end

function Cat:speak(text)
  return 'meow' .. tostring(text)
end
---------Lion------------
local Lion = class(Cat)
function Lion:speak(text)
  return 'Roar...' .. tostring(text)
end

----- Try it -------
local pussycat = Lion("pussycat", "mix-match")

it gives me;

Program starting as '"C:\Users\Public\Lua\ZeroBrainStudio\bin\lua.exe" -e "io.stdout:setvbuf('no')" "C:\Users\Thijs\Desktop\test3.lua"'.
Program 'lua.exe' started in 'C:\Users\Thijs\Dropbox\Lua projects\Penlight' (pid: 2664).
C:\Users\Public\Lua\ZeroBrainStudio\bin\lua.exe: c:\users\public\lua\5.1\share\lua\5.1\pl\class.lua:116: stack overflow
stack traceback:
    c:\users\public\lua\5.1\share\lua\5.1\pl\class.lua:116: in function 'base'
    C:\Users\Thijs\Desktop\test3.lua:26: in function <C:\Users\Thijs\Desktop\test3.lua:25>
    (tail call): ?
    C:\Users\Thijs\Desktop\test3.lua:26: in function <C:\Users\Thijs\Desktop\test3.lua:25>
    (tail call): ?
    C:\Users\Thijs\Desktop\test3.lua:26: in function <C:\Users\Thijs\Desktop\test3.lua:25>
    (tail call): ?
    C:\Users\Thijs\Desktop\test3.lua:26: in function <C:\Users\Thijs\Desktop\test3.lua:25>
    (tail call): ?
    C:\Users\Thijs\Desktop\test3.lua:26: in function <C:\Users\Thijs\Desktop\test3.lua:25>
    ...
    (tail call): ?
    C:\Users\Thijs\Desktop\test3.lua:26: in function 'reset'
    C:\Users\Thijs\Desktop\test3.lua:9: in function '_init'
    c:\users\public\lua\5.1\share\lua\5.1\pl\class.lua:30: in function 'call_ctor'
    c:\users\public\lua\5.1\share\lua\5.1\pl\class.lua:26: in function 'super'
    C:\Users\Thijs\Desktop\test3.lua:21: in function '_init'
    c:\users\public\lua\5.1\share\lua\5.1\pl\class.lua:30: in function 'call_ctor'
    c:\users\public\lua\5.1\share\lua\5.1\pl\class.lua:191: in function 'Lion'
    C:\Users\Thijs\Desktop\test3.lua:69: in main chunk
    [C]: ?
Program completed in 0.78 seconds (pid: 2664).

[Tieske]

It seems to happen with an instance of the 3rd class in the chain; Animal -> Cat -> Lion

In my own usecase I have; step -> test -> pending
And it's initializing a pending class that fails.

[stevedonovan]

Yes, it's a bug in instance:base - which looks at the class of self, and uses its _base field. So our object comes in as a Lion, so the base is Cat - we end up endlessly calling Cat:reset. I'll look at this but I can't see how base can know that it's inside a class X ?

[Tieske]

provide the ancestor as an upvalue? something like this:

change signature of

local function base_method(self,method,...)

into

local function base_method(self, base, method,...)

And then change the implementation to use base instead of grabbing m.

Then when constructing replace

c.base = base_method

with

c.base = function(self, ...) return base_method(self, base, ...) end

I find the code very terse, hence I didn't provide a PR becasue I might do more harm than good.

[stevedonovan]

That's a good solution - I didn't think of using an upvalue! Was about to suggest we trash this cute method, but your scheme will work.

[stevedonovan]

Alas, doesn't fly. Lion.reset is bound to Lion's base, which is Cat. So we end up calling Cat.reset continuously.

The only solution I can think of is expensive, and needs the debug library to find what method we're calling from, and what the class of that method is.

[stevedonovan]

We're doing this all to avoid the straightforward (and fast) Animal.reset(self). Perhaps it's best to suggest this as the best way of accessing a definite base class?

[Tieske]

We're doing this all to avoid the straightforward (and fast) Animal.reset(self). Perhaps it's best to suggest this as the best way of accessing a definite base class?

If that is the solution then it now extends to 3 methods of calling a base class (this method, using super in the _init method, and the base() method), that is basically a mess.

[Tieske]

I don't get the code. Try this code:

-- animal.lua

class = require 'pl.class'

--------Animal----------
local Animal = class()

function Animal:_init(name)
  self:reset()
  self.name = name
end

function Animal:reset()
  self.legs = nil
end

---------Cat------------
local Cat = class(Animal)

function Cat:_init(name,breed)
   self:super(name)  -- must init base!
   self.breed = breed
end

function Cat:reset()
  --self:base("reset")
  self.legs = 4
end

function Cat:speak(text)
  return 'meow' .. tostring(text)
end
---------Lion------------
local Lion = class(Cat)
function Lion:speak(text)
  return 'Roar...' .. tostring(text)
end

----- Try it -------
---[[
local register = {}
local function reg(name, value)
  register[value] = register[value] or name
  if type(value) == "table" and getmetatable(value) then
    reg(name.."_MT", getmetatable(value))
  end
end
local function req(value)
  if register[value] then
    return "---> "..register[value]
  else
    return value
  end
end

local function printit(t, name)
  print(name)
  for k,v in pairs(t) do print("",k,req(v)) end
  if getmetatable(t) then
    print(name.."_MT;", req(getmetatable(t)))
    for k,v in pairs(getmetatable(t)) do print("",k,req(v)) end
  end
end

reg("Animal", Animal)
reg("Cat", Cat)
reg("Lion", Lion)

local pussycat = Lion("pussycat", "mix-match")

reg("pussycat", pussycat)

printit(Animal, "Animal")
printit(Cat, "Cat")
printit(Lion, "Lion")
printit(pussycat, "pussycat")

Then you get the following:

Program starting as '"C:\Users\Public\Lua\ZeroBrainStudio\bin\lua.exe" -e "io.stdout:setvbuf('no')" "C:\Users\Thijs\Desktop\test3.lua"'.
Program 'lua.exe' started in 'C:\Users\Public\Lua\5.1\share\lua\5.1' (pid: 8680).
Animal
    class_of    function: 002EA100
    _init   function: 008CEF78
    cast    function: 008CEF18
    _class  ---> Animal
    __index ---> Animal
    is_a    function: 002EA0B0
    catch   function: 002EA650
    base    function: 002EA178
    reset   function: 008CC280
Animal_MT;  ---> Animal_MT
    __call  function: 002E9DF8
Cat
    class_of    function: 002EA100
    _init   function: 008CC500
    speak   function: 002DDE98
    cast    function: 008CEF18
    catch   function: 002EA790
    _base   ---> Animal
    base    function: 002EA178
    is_a    function: 002EA0B0
    _class  ---> Cat
    __index ---> Cat
    reset   function: 008CC520
Cat_MT; ---> Cat_MT
    __call  function: 002E9E30
Lion
    _base   ---> Cat
    __tostring  function: 002EA1A0
    reset   function: 008CC520
    cast    function: 008CEF18
    catch   function: 002E06D8
    __index ---> Lion
    _class  ---> Lion
    is_a    function: 002EA0B0
    class_of    function: 002EA100
    base    function: 002EA178
    speak   function: 002DE138
Lion_MT;    ---> Lion_MT
    __call  function: 002E9E68
pussycat
    legs    4
    name    pussycat
    breed   mix-match
pussycat_MT;    ---> Lion
    _base   ---> Cat
    __tostring  function: 002EA1A0
    reset   function: 008CC520
    cast    function: 008CEF18
    catch   function: 002E06D8
    __index ---> Lion
    _class  ---> Lion
    is_a    function: 002EA0B0
    class_of    function: 002EA100
    base    function: 002EA178
    speak   function: 002DE138
Program completed in 0.05 seconds (pid: 8680).

It seems very complex to me. There are lots of _base, _class and __index fields, often pointing to the same tables/classes.

Can't it be done by simply making the metatable __index field the ancestor? That gives a very straight forward inheritance chain. The difference between a class and an instance would simply be the availability of an __call metamethod that is equal to the constructor function.

Or am I missing something? (probably, because it seems to simple 😄)

[stevedonovan]

These things all start out simple ;) In essence, not complicated:

• contents of base class copied to new class ('fat inheritance'), including _base as pointer to original base
• class gets an MT so it can have __class

But then we wanted convenience (self:super(...)) ways to query type (Lion:class_of(leo)), hooks to catch undefined methods, and so forth.

So, even if we used 'inheritance through chaining' I bet you the result would not look that much simpler...that was a deliberate trade-off, so that long inheritance chains would not be penalized.

A mess? Yes, I'm afraid that the base method should probably be removed if it cannot be made robust. When we try to bring in ideas from more familiar OOP languages there are restrictions, since they will usually encode the exact target class when executing the equivalent of 'base' (usually called 'super'). So a little inconsistency (like Animal.reset(self)) is the 'customs duty' for importing a foreign concept ;)

[Tieske]

I tried several things, but only to conclude that base will not work in this way. I think it has to go...

[Tieske]

While testing I also ran into a bug with super;

local A = class()
function A:_init()
  self.init_chain = "A"
end
local B = class(A)
local C = class(B)
function C:_init()
  self:super()
  self.init_chain = self.init_chain.."C"
end
local D = class(C)
local E = class(D)
function E:_init()
  self:super()
  self.init_chain = self.init_chain.."E"
end
local F = class(E)
local G = class(F)
function G:_init()
  self:super()
  self.init_chain = self.init_chain.."G"
end

local i = G()
assert(i.init_chain == "ACEG")

I get this when running;

Program starting as '"C:\Users\Public\Lua\ZeroBrainStudio\bin\lua.exe" -e "io.stdout:setvbuf('no')" "C:\Users\Thijs\Dropbox\Lua projects\Penlight\tests\test-class.lua"'.
Program 'lua.exe' started in 'C:\Users\Thijs\Dropbox\Lua projects\Penlight' (pid: 6200).
C:\Users\Public\Lua\ZeroBrainStudio\bin\lua.exe: ...s\Dropbox\Lua projects\Penlight\tests\test-class.lua:69: attempt to call method 'super' (a nil value)
stack traceback:
    ...s\Dropbox\Lua projects\Penlight\tests\test-class.lua:69: in function '_init'
    c:\users\public\lua\5.1\share\lua\5.1\pl\class.lua:30: in function 'call_ctor'
    c:\users\public\lua\5.1\share\lua\5.1\pl\class.lua:174: in function 'G'
    ...s\Dropbox\Lua projects\Penlight\tests\test-class.lua:73: in main chunk
    [C]: ?
Program completed in 0.08 seconds (pid: 6200).

This is because G tries (through super) to call the ancestor _init method on F which doesn't have an _init method because it inherits it from E.

[Tieske]

so if the convenience function base doesn't work and has to go, should super then stay? Or should it also go and all ancestor calls will always be in dot-notation as in ancestor_class.method_name(self,...)? I would prefer such a consistent approach.

[stevedonovan]

I think I've rescued super:

-- this trickery is necessary to prevent the inheritance of 'super' and
-- the resulting recursive call problems.
local function call_ctor (c,obj,...)
    -- nice alias for the base class ctor
    local base = rawget(c,'_base')
    if base then
        local parent_ctor = rawget(base,'_init')
        while base and not parent_ctor do
            base = rawget(base,'_base')
            parent_ctor = rawget(base,'_init')
        end
        if parent_ctor then
            rawset(obj,'super',function(obj,...)
                call_ctor(base,obj,...)
            end)
        end
    end
    local res = c._init(obj,...)
    rawset(obj,'super',nil)
    return res
end

That is, go for a hunt for the first base class with _init.

I am trying to save base but the cure seems worse than the disease ;)

[Tieske]

gave it some thought today. An alternative would be to make sure that every class, upon creation, gets an _init method that just contains self:super()

[stevedonovan]

That was my first try. It did work, and was straightforward, But your assert(i.init_chain == "ACEG") failed; the result was actually ACCEEG or something like that. Above fix is more conservative.

By the way, I can no longer remember the purpose of _post_init ;) If I can't think of a good reason for it, it should go. (_class_init is useful, see how the Properties class is defined, but then should it not be inherited, like the not-found hook?)

No longer convinced that the new _create 'pre-constructor' achieves much; a regular _init can return a new object, and the only cost is one lost table (the initial object). Is such an optimization really needed?

[Tieske]

_class_init is useful, see how the Properties class is defined, but then should it not be inherited, like the not-found hook?

Yes it seems so, but how often is this used? my guess is very little (the properties class is probably the only implementation of it). So maybe reduce on the overhead and have the _init method do this?

For me just a class with _init method, the query methods class_of etc. and a_base field will do. Possibly for 5.2 an _destroy method. But then again, I'm a fan of simple, not everybody is...

[stevedonovan]

I am liking the idea of nice and simple at the moment ;) I've just patched super and removed the base method.

Let me look at _class_init and whether it needs to be special