[Parser] Support prologue and epilogue sourcemap annotations #6370
Conversation
and fix a bug with sourcemap annotations on folded `if` conditions. Update IRBuilder to apply prologue and epilogue source locations when beginning and ending a function scope. Add basic support in the parser for explicitly tracking annotations on module fields, although only do anything with them in the case of prologue source location annotations.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| const Annotation* annotation = nullptr; | ||
| for (auto& a : annotations) { | ||
| if (a.kind == srcAnnotationKind) { | ||
| annotation = &a; |
There was a problem hiding this comment.
This is just moved code, but I a question: can there be more than one srcAnnotationKind? (seems like if so then the last wins?)
There was a problem hiding this comment.
Yes and yes. Is there some other existing behavior I should try to match instead?
| (then | ||
| ;; For the new parser | ||
| ;;@ src.cpp:50:1 | ||
| (return) |
There was a problem hiding this comment.
Looks like the old parser doesn't roundtrip this properly and the new one does, is that correct? Perhaps add a comment to clarify that so it's obvious why we have these two 50:1 annotations?
There was a problem hiding this comment.
They can both round-trip properly, but the old one only if the annotation is on the then and the new one only if the annotation is on the return. Do you think this kind of breaking change is going to be an issue? We can make sure j2wasm is doing the right thing, but other users would be more difficult.
There was a problem hiding this comment.
Is it hard to match the old behavior?
This doesn't seem like a dangerous breaking change to me, as our text format in general is not entirely standard nor stable, but we should document it.
There was a problem hiding this comment.
There would be some very ad-hoc plumbing required to carry the annotation from the then to the first instruction inside the then, so I'd like to avoid it if it's not necessary.
There was a problem hiding this comment.
Do you have a suggestion about where to document it? The changelog might make sense, but more so if we were actually enabling the new parser at the same time.
There was a problem hiding this comment.
Yes, the changelog. I agree it makes more sense when we enable the new parser. Maybe we can put a note in the changelog up above the current release notes, in preparation for enabling the new parser, to remind us, or something like that.
and fix a bug with sourcemap annotations on folded
ifconditions. UpdateIRBuilder to apply prologue and epilogue source locations when beginning and ending
a function scope. Add basic support in the parser for explicitly tracking
annotations on module fields, although only do anything with them in the case of
prologue source location annotations.