-
Notifications
You must be signed in to change notification settings - Fork 353
Implement power assertions #1384
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
Conversation
This adds syntax highlighting of Pkl code! It adds highlighting for: * Stack frames within error messages * CLI REPL (highlights as you type, highlights error output) * Power assertions (coming in #1384) This uses the lexer for highlighting. It will highlight strings, numbers, keywords, but doesn't understand how to highlight nodes like types, function params, etc. The reason for this is because a single line of code by itself may not be grammatically valid.
HT154
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!! Just a few minor things.
pkl-core/src/main/java/org/pkl/core/ast/type/VmTypeMismatchException.java
Outdated
Show resolved
Hide resolved
pkl-core/src/main/java/org/pkl/core/runtime/PowerAssertions.java
Outdated
Show resolved
Hide resolved
stackoverflow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
Just some small things and clarifications.
| } catch (ex: Exception) { | ||
| errWriter.appendLine("Error evaluating module ${moduleUri.path}:") | ||
| errWriter.write(ex.message ?: "") | ||
| errWriter.write(ex.stackTraceToString()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intended to be here, or is it a debug leftover?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional, but this is a bugfix unrelated to this feature. I'll pull this into a separate PR
9f15209 to
072eaa6
Compare
stackoverflow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This annotates failing assertions in type constraints, and in tests with the values produced during code execution. This follows the power assertions style of reporting also found in Groovy, Kotlin, and others. * Literal values are not emitted in the value graph * Stdlib constructors of literals like `List(1, 2)` are also considered literals
Co-authored-by: Islon Scherer <[email protected]>
c5f05c4 to
3d7d6c7
Compare
This adds power assertions to Pkl!
This implements the SPICE described in apple/pkl-evolution#29
This follows the power assertions style of reporting also found in
Groovy, Kotlin, and others.
List(1, 2)are also consideredliterals
Power assertions are added to:
Power assertions are implemented as a truffle instrument to observe execution.
When an assertion fails, the instrument is created and the assertion is run again to observe facts.
This incurs runtime overhead to collect facts, but has no impact on code in the non-error case.
Samples: