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

Printing with metatable's "__tostring" if userdata has it? #38

Open
eliasdaler opened this issue Apr 11, 2018 · 9 comments
Open

Printing with metatable's "__tostring" if userdata has it? #38

eliasdaler opened this issue Apr 11, 2018 · 9 comments

Comments

@eliasdaler
Copy link

Hello. First of all, I want to thank you for a wonderful lib that I've used for several years now.

I've got an idea. Sometimes it's convenient to add "__tostring" to userdata's metatable so that it's printable in Lua. For example, if I bind my C++ 2D Vector class, it then can be easily printed like this:

v = Vector2f.new(10, 20) -- create instance of C++ Vector
print(tostring(v)) -- prints "(10, 20)"

But when I print it with inspect, I get:
<userdata 1>

But what if inspect printed something like this if __tostring is present in userdata's metatable?

<userdata 1, tostring = "(10, 20)">

And instead of getting output like this:

{
  room = {
    areaName = "forest",
    size = <userdata 1>
  }
}

I'll be able to get this:

{
  room = {
    areaName = "forest",
    size = <userdata 1, tostring="(10,20)">
  }
}

... Which will greatly help me when debugging some tables.
Another possibility is to make inspect write tables like this:

{
  room = {
    areaName = "forest",
    size = <userdata 1> -- (10, 20)
  }
}

Maybe this can be turned on and off with options if you don't like this behavior to be present by default. I can make a PR with my implementation if you agree that this might be a good feature to have.

@eliasdaler eliasdaler changed the title Printing "__tostring" if userdata has it? Printing with metatable's "__tostring" if userdata has it? Apr 11, 2018
@kikito
Copy link
Owner

kikito commented Apr 12, 2018

Hi! You must be using a very recent version of inspect.lua, because I removed that functionality from it on my last commit, 6 days ago.

Explanation: I had exactly that functionality before, but I decided to remove it when I realized that it precluded using inspect on a table's __tostring method. If you do:

local t = setmetatable({}, { __tostring = inspect })

print(t)

On the previous version this would enter an infinite loop (the first tostring will call inspect, which will call tostring again to generate the comment like you mentioned, and that will call inspect again).

The reason why I didn't release a new version of inspect.lua after I made this change is that I'm still thinking about how to best resolve this problem. I'm open to suggestions.

For now, if you need that feature right away, I recommend you download v3.1.1.

EDIT: upon re-reading your issue, it seems that you want to include it only on userdata. I will think about this as well.

@eliasdaler
Copy link
Author

Yes, I mainly want it for userdata as it should be safe from this. As for usual tables - I'll try to work out a way to solve this problem.

Btw, this code works fine (if I checkout to this commit):

local inspect = require 'inspect'
local t = setmetatable({f = 5}, { __tostring = inspect })
print(t)

But this one enters the infinite loop (you probably meant this one, right?):

local inspect = require 'inspect'
local t = setmetatable({f = 5}, { __tostring = inspect.inspect })
print(t)

@eliasdaler
Copy link
Author

Seems like I've fixed it. See my tostring-fix branch on my fork and feel free to point out cases where it may fail.

Here's what I've used for testing:

local inspect = require 'inspect'
local t = setmetatable({}, { __tostring = inspect.inspect })

print("> print(t)")
print(t)
print('\n')

print("> print(inspect(t))")
print(inspect(t))
print('\n')

local t2 = setmetatable({}, { __tostring = function() return "ok" end })

print("> print(t2)")
print(t2)
print('\n')

print("> print(inspect(t2))")
print(inspect(t2))

This outputs:

> print(t)
{
  <metatable> = {
    __tostring = <function 1>
  }
}


> print(inspect(t))
{
  <metatable> = {
    __tostring = <function 1>
  }
}


> print(t2)
ok


> print(inspect(t2))
{ -- ok
  <metatable> = {
    __tostring = <function 1>
  }
}

@kikito
Copy link
Owner

kikito commented Apr 12, 2018

Hi, thanks for giving this a try! Unfortunately it only solves the most obvious case - if you wrap inspect in a function it will still enter infinite loops:

local f = function(x) return inspect(x) end
local t = setmetatable({}, { __tostring = f })
print(t)

I don't think this can be completely solved if you want to execute __tostrings at all. Maybe the right
solution is just using a config option and make it default to false. I still have to think about it.

@eliasdaler
Copy link
Author

eliasdaler commented Apr 12, 2018

Can you check out the latest commit at my branch? Seems like the problem you pointed out is fixable, but possibly has some problems with it... (ah, the wonderful world of recursion).

local f = function(x) return inspect(x) end
local t = setmetatable({}, { __tostring = f })
print(t)

now prints:

{ -- {\n  <metatable> = {\n    __tostring = <function 1>\n  }\n}
  <metatable> = {
    __tostring = <function 1>
  }
}

We can also make some error for user doing it instead of showing this. Or maybe we can make the output better somehow...

P.S. It may be worth leaving this safe guarding in the code even if this __tostring stuff is optional. Will probably save someone from nasty crash.

@eliasdaler
Copy link
Author

eliasdaler commented Apr 12, 2018

... and this won't work in this case:

local inspect = require 'inspect'
local t2 = setmetatable({}, { __tostring = function() return "HEHE" end })

local f = function(x)
    inspect(t2) -- this will reset inspect.GUARD :(
    return inspect(x)
end

local t = setmetatable({}, { __tostring = f })
print(t)

And we can go further - what if __tostring calls a C function which calls inspect on the same table... and so on.

I don't know, maybe it's better to make __tostring behaviour optional and warn users that they're gonna have a bad time if their __tostring calls inspect...

@hishamhm
Copy link

hishamhm commented Apr 5, 2019

@kikito, can we have this for userdata only? It is unlikely that __tostring of userdata will ever be inspect, because that has no useful behavior.

One out-of-the-box benefit of doing this is identifying ngx.null in OpenResty applications (I've seen colleagues get stumped by this when debugging with inspect).

hishamhm added a commit to hishamhm/inspect.lua that referenced this issue Apr 5, 2019
@roman-orekhov
Copy link

On a related note -- why isn't inspect showing metatables for userdatas?

@svermeulen
Copy link

I really wanted inspect to use __tostring, so I made this modification. It will use __tostring, and if it encounters a recursive attempt to call inspect again it will throw an error

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

No branches or pull requests

5 participants