fix linenumbers in failed assertion traces #380
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
/claim #354
There are two places that break line numbering:
1. Tracer. apply - betaReduce
compiler implementation of BetaReduce tries to convert Inlined terms using inlineContext - which seems to break positions
There are 2 ways to fix:
a - do not call betaReduce at all
b - transform Inlined with current Quotes context
I have chosen b - because it does a lesser logic change compared to b.
My implementation is equivalent to what Term.betaReduce does, with one difference that original Term.betaReduce computes inlineContext instead of directly using Quotes.
2. TestBuilder.testCallTreeExpr
Despite
assert
expression does have proper position attached to it, compiler for some reasons doesn't use it in generation of lineNumbersTable in class files.Wrapping it into
Inlined
term fixes it.Honestly, I didn't dig into compiler internals to figure out how exactly it treats
Inlined
when generating LineNumberTable.However have seen lots of places where
Inlined
terms get some special treatment when it comes to spans (which are responsible for linenumbers and positions).Alternative is to manually control stack traces by editing StackTraceElements, which may be more flexible, I can implement it as well - just lmk which in your opinion is better way to go
Worth to mention:
If we use only
assert
macro then fix 1 is enough, however when we use bothTest
andassert
inside it - which is normal usecase of utest, then two fixes are needed.Inlined.copy
orInlined.apply
to have some influence on linenumbers, but its limited.changeOwner
doesnt affect linennumber