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

Improve output of userdata, handling__tostring and NULL #45

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hishamhm
Copy link

@hishamhm hishamhm commented Apr 5, 2019

This PR includes 2 commits:

The first one includes evaluation of __tostring for userdata values. This should be safe because __tostring of userdata is unlikely to recurse into inspect. A test is included.

The second one adds a special case for NULL userdata.

When interpreting userdata values using 'inspect', the numeric identifiers
are useful to match identify of pointers (comparing the low counters
like <userdata 3> is easier than reading long pointers like userdata: 0x749abc39efa29`).

However, the NULL pointer is enough of a special case that it is
important to know when one of those numbered pointers happens to
be NULL. When one is dealing with things like JSON parsers, where
the only usual userdata is cjson.null, one gets accostumed over
time to interpret <userdata 1> to mean NULL. This is not obvious,
however, and a seasoned user will trip up the day another userdata
is used in the same table and NULL is now <userdata 2>.

Adding a test for this would require compiling and loading a C extension, such as lua-cjson. I can do so if desired.

When interpreting userdata values using 'inspect', the numeric identifiers
are useful to match identify of pointers (comparing the low counters
like `<userdata 3> is easier than reading long pointers like
`userdata: 0x749abc39efa29`).

However, the `NULL` pointer is enough of a special case that it is
important to know when one of those numbered pointers happens to
be `NULL`. When one is dealing with things like JSON parsers, where
the only usual userdata is `cjson.null`, one gets accostumed over
time to interpret `<userdata 1>` to mean `NULL`. This is not obvious,
however, and a seasoned user will trip up the day another userdata
is used in the same table and `NULL` is now `<userdata 2>`.

Adding a test for this would require compiling and loading a C extension.
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.1%) to 98.178% when pulling c32cdda on hishamhm:tostring-userdata into b611db6 on kikito:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.1%) to 98.178% when pulling c32cdda on hishamhm:tostring-userdata into b611db6 on kikito:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.1%) to 98.178% when pulling c32cdda on hishamhm:tostring-userdata into b611db6 on kikito:master.

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

Successfully merging this pull request may close these issues.

2 participants