-
Notifications
You must be signed in to change notification settings - Fork 158
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
feat: add assembly SourceLocation #861
Conversation
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.
Thank you! I left a couple of test-related comments inline, but I have a more general question:
What is the reason for introducing TokenLines
and SourceToken
structs? It seems to me that to support mapping of tokens to source locations we just need to replace these lines with some parse_line_tokens()
function which would return a list of tokens together with their SourceLocation
's. Or does this complicate things for some reason?
8b70fd1
to
3f910b3
Compare
3f910b3
to
7ad7146
Compare
3cd7652
to
46619ee
Compare
cfb8a23
to
de07679
Compare
This commit introduces [SourceLocation], a structure linked to a [Token] via [TokenStream]. It will allow the link between a MASM source and a parsed [Operation]. related issue: #857
de07679
to
fb84dd9
Compare
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.
Thank you! Looks good! I left a couple of small comments inline.
Just wanted to throw my two cents in, because I've implemented this kind of infrastructure a few times before, have done so for the compiler already, and @grjte and I discussed extracting that crate out of the For some additional context on how this generally gets used with a frontend, this parsing infrastructure builds on top of that diagnostics crate, and can be used with At a high-level, the
All of the above is oriented primarily towards the frontend of a compiler/interpreter (i.e. used during parsing/compilation), but it can be easily integrated into emission of a more stable debug info format in a variety of different ways depending on what properties you want from the debug info. For example, you could serialize the entire Emitting Diagnostics#[derive(Spanned)]
struct Foo {
#[span]
span: SourceSpan,
}
impl Foo {
fn compile(&self, diagnostics: &DiagnosticsHandler) -> anyhow::Result<()> {
...
match bar() {
ok @ Ok(_) => ok,
Err(err) => {
diagnostics.diagnostic(Severity::Error)
.with_message(format!("failed to bar: {}", &err))
.with_primary_label(self.span(), "in this foo")
.emit()
Err(err)
}
}
} |
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.
All looks good! Thank you!
@bitwalker - this looks cool! It might be too big of a lift to integrate this into the assembler in a single go - but I think we'll need to use something like |
@bitwalker yes, using the We should target using a centralized implementation too, but let's see a bit down the road |
This commit introduces [SourceLocation], a structure linked to a [Token] via [TokenStream].
It will allow the link between a MASM source and a parsed [Operation].
related issue: #857