Fix issue with lua_loadfile and require.#2317
Conversation
tritao
force-pushed
the
fix-loadfile-require
branch
from
eed8e2d to
e28985b
Compare
Fixes an regression introduced in premake@afcbbb2. `require` actually follows a different code path in Lua with a different stack layout expectation than the other code paths. Fix the code to deal with this case, and also clean up some other tidbits around cleaning up stack values. The new approach is simpler to understand and should fix a potential issue that could happen in the error case. Fixes premake#2314.
tritao
force-pushed
the
fix-loadfile-require
branch
from
e28985b to
7fcf9dd
Compare
| * script chunk on the stack. Turn these into a closure that will call my | ||
| * wrapper below when the loaded script needs to be executed. */ | ||
|
|
||
| assert(lua_gettop(L) == bottom + 2); |
There was a problem hiding this comment.
Do we want an assertion failure here, or should we report the error more gracefully somehow?
There was a problem hiding this comment.
I think assert is probably the correct thing to do here, this is basically a sanity check on my part. Assert messages are only fired up in debug mode anyway, so should make no difference to users.
There was a problem hiding this comment.
If we end up having any more issues here, I can look into doing something more involved but for now I think this is good enough.
|
Please go ahead and ping me whenever you push up for this/reply to comments. Once we get this in, I'll be getting a release together. |
Replied to your comment. @nickclark2016 |
3 participants
Fixes an regression introduced in afcbbb2.
requireactually follows a different code path in Lua with a different stack layout expectation than the other code paths.Fix the code to deal with this case, and also clean up some other tidbits around cleaning up stack values. The new approach is simpler to understand and should fix a potential issue that could happen in the error case.
Fixes #2314.
Did you check all the boxes?
closes #XXXXin comment to auto-close issue when PR is merged)You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!