-
Notifications
You must be signed in to change notification settings - Fork 2
Conversation
This reverts commit d748797.
Marking this for review without doing tokens for attributes. Attributes don't have any location information in the kast format, so supporting it would require more work than just getting locations on these tokens. |
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.
I appreciate the work you put in to update the tests, thank you!
@@ -53,25 +61,33 @@ BUBBLE | |||
</k> | |||
|
|||
LBRACK | |||
1,0 |
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 doesn't look right.
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 is one of the attribute tokens, which aren't getting location info because attributes don't have source locations in KAST, so it would be extra work in this PR for nothing. Instead, they're getting the default INIT_LOC
.
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.
It is not for nothing, it enables more fine-grained error reporting for attributes. Please open an issue so that we can address this later.
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.
In that issue, please also include that we should add the following test.
After lexing, for each token, if we consider the substring of the input text starting at the token location, it will start with the token text.
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.
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.
Looks great!
Converting this to draft to avoid accidentally merging it before the repo merge. |
Transfer of: runtimeverification/pyk#1052 --------- Co-authored-by: Guy Repta <[email protected]>
runtimeverification/k#4180