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

Better syntax errors #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Frityet
Copy link
Contributor

@Frityet Frityet commented Jul 4, 2024

Adding the message at the top of the stack lets you know what failed

@hugomg
Copy link
Owner

hugomg commented Jul 5, 2024

I'm confused why LuaAOT didn't detect this as a compile-time error. In theory the first thing we do is compile to bytecode and then turn that bytecode into C. Shouldn't the syntax error have been caught in that first step?

ps.: I'm currently working on a branch to update LuaAOT to Lua 5.4.6. After that, there will likely be merge conflicts for this PR. (hopefully not much)

@Frityet
Copy link
Contributor Author

Frityet commented Jul 25, 2024

I'm confused why LuaAOT didn't detect this as a compile-time error. In theory the first thing we do is compile to bytecode and then turn that bytecode into C. Shouldn't the syntax error have been caught in that first step?
Yes, but shebangs are valid lua, and can't be interpreted with loadstring. Just in case there are other cases like that I think its useful to have the message

I'm currently working on a branch to update LuaAOT to Lua 5.4.6
awesome, thank you so much!

@hugomg
Copy link
Owner

hugomg commented Jul 25, 2024

Yes, but shebangs are valid lua, and can't be interpreted with loadstring. Just in case there are other cases like that I think its useful to have the message

Looks like the magic is all in luaL_loadfilex. All it does is eat an optional UTF byte order mark and shebang. See functions skipBOM and skipcomment in lauxlib.c

Since there are no other cases to worry about, I think it makes sense to replicate the behavior of skipBOM and skipcomment. We can keep a sanity check to see if loadstring had a syntax error or not, but we don't need to present a pretty user-facing message because in theory that should never happen.

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