Penlight icon indicating copy to clipboard operation
Penlight copied to clipboard

The `:catch()` method in pl.class does not pass self correctly

Open alerque opened this issue 6 years ago • 10 comments

This fails:

local myClass = class({
  _init = function (self)
    self:catch(self.lookup)
  end,
  lookup = function (self, key)
    --- self will be an empty table empty here
  end
})

This works:

local myClass = class({
  _init = function (self)
    self:catch(function(_, k) return self:lookup(k) end)
  end,
  lookup = function (self, key)
    --- self will actually be self here
  end
})

Note that two arguments are getting passed an the property name to try to find is the second argument, so I would have expected the first argument to be self. This is not the case. Not that this contradicts the example in the documentation:

It’s definitely more useful to define exactly how your objects behave in unknown conditions. All classes have a catch method you can use to set a handler for unknown lookups; the function you pass looks exactly like the __index metamethod.

Strings:catch(function(self,name)
  return function() error("no such method "..name,2) end
end)

If it was really the same as __index() then self should be passed through as the first argument.

alerque avatar Sep 27 '19 09:09 alerque

~~The same thing is happening with setting __tostring() in a class. The meta method gets set and called as expected, but the self value in empty.~~

Scratch that, the problem is related but not the same. With __tostring() the self is getting passed, but it cannot be queried with rawget() for some reason.

alerque avatar Sep 27 '19 10:09 alerque

is this related to #101 ?

Tieske avatar Jul 30 '20 11:07 Tieske

I don't think I was using class.properties at all (to my recollection I never have). That doesn't mean they are not a related weakness in :catch(), but they don't sound like the same problem to me.

alerque avatar Jul 30 '20 11:07 alerque

all of those are somehow related, see https://github.com/Tieske/Penlight/issues/79 essentially the class thing is broken beyond repair...

Tieske avatar Jul 31 '20 10:07 Tieske

I wouldn't say the class system as a whole is broken beyond repair, I use it extensively in several active projects. There are just a couple aspects of classes that don't work right and must be avoided.

alerque avatar Jul 31 '20 11:07 alerque

those 2 statements are equivalent; yours from a user perspective (avoid what doesn't work), mine from an owner perspective (can't possibly keep the promises made to the user)

Tieske avatar Jul 31 '20 11:07 Tieske

This almost certainly has to do with instance.catch as defined by a new class definition being clobbered by the default one during the class definition phase: a default function is being defined after the user input model methods are copied into the target class table. For some of the alternate instantiation syntax options.

Additionally my example code above is screwing, probably because I was trying to work around one of the other class related bugs at the same time.

alerque avatar Sep 29 '20 15:09 alerque

Having spend almost a week of free time trying to root out bugs downstream of this problem, the only thing I have to add for now is this...

Both methods, self:catch(self:getter) and self:catch(function(_, k) return self:getter(k) end) are activated as an unknown key handler, both get the same k, and both get the same content in the self table. The main difference I can find is that the table ID of self is different, somehow it has been copied or moved into a different place on the stack.

function myclass:getter (k)
	print(string.format("%p"), self)
end

For the first method shown above, this will be the same id shown in myclass:_init and any other time you check the instance of myclass. For the second method it will be some different id.

alerque avatar Jan 26 '22 08:01 alerque

is it a single class, or does it inherit? self.super can never work

Say you have "animal -> cat -> lion" inheritance chain.

  • Instance of lion accesses a method defined on lion, which accesses self.super -> super returns "cat class"
  • Instance of lion accesses a method defined on cat, which accesses self.super -> super returns "cat class" again! but should have been "animal class", since the calling method was defined on "cat".

even if super is not a property, but a dynamic value retrieved/lookedup by the __index method, that method would have no way of determining from what class the call was made, so how far up in the chain it should call the ancestor.

My guess is you're being bitten by this, and the different "self" you get is one of the ancestors, but not the right one.

This problem is what led me to: https://github.com/lunarmodules/Penlight/issues/307#issuecomment-667045456

edit: typo, wrong class name

Tieske avatar Jan 27 '22 13:01 Tieske

No, in this case I'm using a shallow model with no more than 1 level of inheritance. I have one base class and everything else is variants inheriting only from that one class, there are no extra levels going on.

BTW I plan of fixing the depth problem because it is very restricting, but for this case I also need compatibility with old Penlight versions so I'm kind of stuck with the broken model for now.

alerque avatar Jan 27 '22 14:01 alerque