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

Hints for runtime errors #2097

Merged
merged 1 commit into from
Feb 13, 2025
Merged

Hints for runtime errors #2097

merged 1 commit into from
Feb 13, 2025

Conversation

SquidDev
Copy link
Member

@SquidDev SquidDev commented Feb 9, 2025

Only two years after originally suggested, this adds hints for misspelled globals and table keys:

A CC computer. The user has tried to run redstone.getinput(), and we've printed a "Did you mean getInput" after the error.

This is heavily WIP. For one, the above screenshot is heavily cherry-picked. The suggestions are pretty broken in many cases — I'd forgotten how much I hate string distance metrics. We're currently using Jaro–Winkler, but we probably want to switch to (Damerau–)Levenshtein, and maybe play around with weights and whatnot.

Similar to the above, but redstone.getoutput suggests both getOutput and getAnalogOutput. Even worse, redstone.getouput has no suggestions.

Perhaps more importantly, I am incredibly unsure about the implementation here. The core logic is written in Java — we inspect (and partially evaluate) the Lua bytecode to find the failing operation and its operands.

There's a couple of things I'm not a fan of here:

  • It feels a bit magic to add a Java module for this. I'm not entirely adverse to it — this is never going to be part of the public API, so we can rip it out again if needed — but it does feel pretty ugly!
  • Lua bytecode doesn't store the full list of names in scope (only locals, and upvalues which are captured), so any attempt to global "foo" errors will have incomplete/incorrect suggestions.

The alternative here would be to parse the code in Lua, and evaluate the expression that way. However, this also has some issues:

  • This incurs a pretty significant performance. I'm particularly concerned about memory, as we'd have to realise the whole parse tree. I don't think there's a way we can (easily) lazily do this.
  • Anything more advanced then what we're doing right now would require proper semantic analysis of the code. But that's probably the case with either approach, so maybe that's fine!

@SquidDev SquidDev added enhancement An extension of a feature or a new feature. area-CraftOS This affects CraftOS, or any other Lua portions of the mod. labels Feb 9, 2025
We now suggest alternative table keys when code errors with "attempt
to index/call 'foo' (a nil value)". For example: "redstone.getinput()",
will now suggest "Did you mean: getInput".

This is a bit tricky to get right! In the above example, our code reads
like:

   1    GETTABUP 0 0 0 ; r0 := _ENV["redstone"]
   2    GETFIELD 0 0 1 ; r0 := r0["getinput"]
   3    CALL 0 1 1     ; r0()

Note, that when we get to the problematic line, we don't have access to
the original table that we attempted to index. In order to do this, we
borrow ideas from Lua's getobjname — we effectively write an evaluator
that walks back over the code and tries to reconstruct the expression
that resulted in nil.

For example, in the above case:
 - We know an instruction happened at pc=3, so we try to find the
   expression that computed r0.
 - We know this was set at pc=2, so we step back one. This is a GETFIELD
   instruction, so we check the key (it's a constant, so worth
   reporting), and then try to evaluate the table.
 - This version of r0 was set at pc=1, so we step back again. It's a
   GETTABUP instruction, so we can just evaluate that directly.

We then use this information (indexing _ENV.redstone with "getinput") to
find alternative keys (e.g. getInput, getOutput, etc...) and then pick
some likely suggestions with Damerau-Levenshtein/OSD.

I'm not entirely thrilled by the implementation here. The core
interpretation logic is implemented in Java. Which is *fine*, but a)
feels a little cheaty and b) means we're limited to what Lua bytecode
can provide (for instance, we can't inspect outer functions, or list all
available names in scope). We obviously can expand the bytecode if
needed, but something we'd want to be careful with.

The alternative approach would be to handle all the parsing in
Lua. Unfortunately, this is quite hard to get right — I think we'd need
some lazy parsing strategy to avoid constructing the whole AST, while
still retaining all the scope information we need.

I don't know. We really could make this as complex as we like, and I
don't know what the right balance is. It'd be cool to detect patterns
like the following, but is it *useful*?

    local monitor = peripheral.wrap("left")
    monitor.write("Hello")
        -- ^ monitor is nil. Is there a peripheral to the left of the
        -- computer?

For now, the current approach feels the easiest, and should allow us to
prototype things and see what does/doesn't work.
@SquidDev SquidDev force-pushed the feature/error-hints branch from e419318 to cffbd61 Compare February 13, 2025 21:47
@SquidDev SquidDev marked this pull request as ready for review February 13, 2025 21:47
@SquidDev SquidDev enabled auto-merge (squash) February 13, 2025 21:48
@SquidDev
Copy link
Member Author

SquidDev commented Feb 13, 2025

I'm still not wild about the implementation, but as mentioned in the commit message, I think it's worth getting in and seeing how well it does/doesn't work (and hoping it doesn't break!).

@SquidDev SquidDev added the area-Core This affects CC's core (the Lua runtime, APIs, computer internals). label Feb 13, 2025
@SquidDev SquidDev merged commit b185d08 into mc-1.20.x Feb 13, 2025
6 checks passed
@SquidDev SquidDev deleted the feature/error-hints branch February 13, 2025 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Core This affects CC's core (the Lua runtime, APIs, computer internals). area-CraftOS This affects CraftOS, or any other Lua portions of the mod. enhancement An extension of a feature or a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant