-
Notifications
You must be signed in to change notification settings - Fork 10
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
Forbid newindex for non-tables without __newindex #99
Conversation
Previously, a newindex event on a non-table without a suitable __newindex entry in its metatable would be silently ignored. This commit causes an error to be raised.
Codecov ReportBase: 87.72% // Head: 87.72% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## lua5.4 #99 +/- ##
==========================================
- Coverage 87.72% 87.72% -0.01%
==========================================
Files 107 106 -1
Lines 11475 11470 -5
==========================================
- Hits 10067 10062 -5
+ Misses 1088 1086 -2
- Partials 320 322 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This looks good - it causes the Lua test suite to fail though (see https://github.com/arnodel/golua/actions/runs/4152159172/jobs/7191272897#step:8:188). The line that fails is here: https://github.com/arnodel/golua-tests/blob/golua-5.4/coroutine.lua#L384. I didn't have time to find out why. |
Yes, I'll look into it later. On my local machine, the tests are successful. |
At first glance, it just seems like something should have been commented out but wasn't: -- No weak tables in Golua, so removing this check
-- local C = {}; setmetatable(C, {__mode = "kv"}) (https://github.com/arnodel/golua-tests/blob/golua-5.4/coroutine.lua#L373) The question is, why does this not fail everywhere. But as I said, I'll look into it in more detail later. |
This test is not run by `go test ./...`. It is a modified version of the
"official" Lua test suite (https://www.lua.org/tests/). To run it you need
to clone the repository and run golua on it (see
https://github.com/arnodel/golua/blob/lua5.4/.github/workflows/go.yml#L45-L54
for details)
…On Mon, 13 Feb 2023 at 10:19, Alexander Klauer ***@***.***> wrote:
Yes, I'll look into it later. On my local machine, the tests are
successful.
—
Reply to this email directly, view it on GitHub
<#99 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMJKOIJR44SL2VIHURWBRDWXIDETANCNFSM6AAAAAAUYYY5HQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Thanks @TheCount for fixing the issue in golua-tests. All tests now green! |
Previously, a newindex event on a non-table without a suitable __newindex entry in its metatable would be silently ignored. This commit causes an error to be raised.
From the spec:
If none of these conditions applies, an error should be raised.