Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.56%. Comparing base (f1018ee) to head (c27a6ad).
Report is 441 commits behind head on master.

@@            Coverage Diff             @@
##           master   #16989      +/-   ##
==========================================
+ Coverage   98.54%   98.56%   +0.02%     
==========================================
  Files         169      169              
  Lines       21877    21945      +68     
==========================================
+ Hits        21558    21631      +73     
+ Misses        319      314       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +8 +0.001% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

Related to #16797?

Hmm, actually, looking at the compilation result, it looks like gcc -O3 isn't quite strong enough to make the whole result.info_increment part fully transparent --- at least in ARM, there's two extra opcodes with it, associated with moving around that 1 or 2 increment value and incrementing the line_info value with it.

https://godbolt.org/z/hqoYodEaE

There we are, having it return the new line_info value instead of an increment value for it manages to get it fully elided: https://godbolt.org/z/bEY6ffTK8

Though, that makes it much less clear what's going on --- and there's similar differences in the other contexts with how incrementing bc and the like gets rolled in; doing the same thing for them makes it even harder and also ends up pessimizing the mp_bytecode_get_source_line to something even larger than without it.

(Also, clang gets the exact same compilation result regardless of whether or which way I refactor it.)

I'll leave the PR as-is unless we really feel like it's important to turn the screws as tight as possible just to win back 2 opcodes here.

The thread/thread_gc1.py test failure in unix port / settrace_stackless (pull_request) seems to be spurious.

Was initially concerned if there was some sort of overflow making mp_obj_malloc(mp_obj_colines_iter_t, &mp_type_polymorph_iter); somehow a leak (with the fact that mp_obj_colines_iter_t is a lot larger than the mp_obj_iter_buf_t type used for this in mp_obj_tuple_getiter) --- but that test doesn't even interact with code objects, so idk.

And it would be good to add another test to this PR, to test the actual numeric values from co_lines(), to make sure they match the expected bytecode offsets for a simple test function (that test would need a corresponding .py.exp file because you can't use CPython to work out the right answer).

I've been playing around with these tests; at the moment the test I added also covers the cases where a correct co_lines implementation might need to sum together multiple successive table entries to get the correct answer --- which it currently doesn't do.

The current outputs from co_lines are still meaningful and correct in some sense, though --- is that summing behavior actually something this needs to implement? Or can these extra coverage cases just be dropped (or made into cpydiffs or something)?

Also, should I add coverage for a case where the table needs to skip more than 2048 lines at once? (e.g. to overflow the 11-bit line number increment in the 2-byte format, and require a multi-entry encoding for it like bc+0, line+2047; bc+4, line+12)