Skip to content

Conversation

pawelsadlo
Copy link
Contributor

@pawelsadlo pawelsadlo commented Jun 30, 2025

/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:

  • Term.pos is not always equivalent to final line number in the classfiles LineNumberTable.
  • general rule of thumb is that expressions present in source code should get linenumber from sourcecode and expressions generated by macro should get linenumber of where macro is expanded.
    If we use only assert macro then fix 1 is enough, however when we use both Test and assert inside it - which is normal usecase of utest, then two fixes are needed.
  • We cannot change Tree position directly, there is no public api exposed for this. We can try to use Inlined.copy or Inlined.apply to have some influence on linenumbers, but its limited.
  • changeOwner doesnt affect linennumber

@pawelsadlo pawelsadlo marked this pull request as ready for review June 30, 2025 11:17
@lihaoyi
Copy link
Member

lihaoyi commented Jun 30, 2025

I tried this out manually using the example project in https://mill-build.org/mill/dev-1.0.0-RC2-68-43069c/scalalib/intro.html#_simple_scala_module. Updated to Scala 3.7.1 it reproduces the problem, but using a locally-published version of uTest built off of 8b07406 doesn't seem to fix it. Could you take a look and see if that's the case for you as well?

@pawelsadlo
Copy link
Contributor Author

I tried this out manually using the example project in https://mill-build.org/mill/dev-1.0.0-RC2-68-43069c/scalalib/intro.html#_simple_scala_module. Updated to Scala 3.7.1 it reproduces the problem, but using a locally-published version of uTest built off of 8b07406 doesn't seem to fix it. Could you take a look and see if that's the case for you as well?

my bad, assumed that assert is the last line - it should work now, PTAL

@lihaoyi
Copy link
Member

lihaoyi commented Jun 30, 2025

Thanks! I tested it and it seems to work great. Will merge it and pay out the bounty using the same bank details as we used before

@lihaoyi lihaoyi merged commit a8a22b6 into com-lihaoyi:master Jun 30, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants