Add supervisor.get_previous_traceback()

[cwalther]

As discussed in #1084, when a VM run ends with an exception, this uses the movable allocation system to preserve the traceback across a soft reload and lets the next VM run retrieve it using supervisor.get_previous_traceback().

Only the last commit is relevant, the next to last is #3454 (which it is not dependent on and could be rebased off if desired) and the rest is #3695 (which is required).

Open questions from my point of view:

• Should this be optional, or is the added weight in code size and RAM usage considered worth it for all ports?
• Should a REPL run clear the stored traceback? (It can’t end with an exception itself.) This implementation ignores REPL runs because that seemed to make sense on paper, but once implemented I found that behavior surprising myself at one point, so I’m now inclined toward changing it (which would also be a simplification, removing a special case). Or can anyone think of a use for it?
• Please take a close look at what I’m doing in supervisor_get_previous_traceback(). As far as I understand, it works correctly, but my understanding of how the garbage collector works and how strings work may be incomplete. Also, am I inappropriately using non-API (implementation details) of the string class by assembling an mp_obj_str_t object by hand? There doesn’t seem to be a macro/function for what I want.
• Adding a new C file for a single variable feels like overkill, but it didn’t seem to fit in any of the existing ones. Any opinions?

[tannewt]

As discussed in #1084, when a VM run ends with an exception, this uses the movable allocation system to preserve the traceback across a soft reload and lets the next VM run retrieve it using supervisor.get_previous_traceback().

Only the last commit is relevant, the next to last is #3454 (which it is not dependent on and could be rebased off if desired) and the rest is #3695 (which is required).

Ok great! Here are my thoughts on below. I haven't read through the PR yet. I'm starting with #3695.

Open questions from my point of view:

• Should this be optional, or is the added weight in code size and RAM usage considered worth it for all ports?

I think we'll want it optional. It'll be clearer how much flash it costs after the other two PRs are merged.

• Should a REPL run clear the stored traceback? (It can’t end with an exception itself.) This implementation ignores REPL runs because that seemed to make sense on paper, but once implemented I found that behavior surprising myself at one point, so I’m now inclined toward changing it (which would also be a simplification, removing a special case). Or can anyone think of a use for it?

Whatever is simpler makes sense to me. REPL use implies someone is "at the terminal" so I think it's less important. This is mostly for subsequent code to be able to recover on its own.

• Please take a close look at what I’m doing in supervisor_get_previous_traceback(). As far as I understand, it works correctly, but my understanding of how the garbage collector works and how strings work may be incomplete. Also, am I inappropriately using non-API (implementation details) of the string class by assembling an mp_obj_str_t object by hand? There doesn’t seem to be a macro/function for what I want.

I'll try and remember to do this. Generally, I think of all C APIs as private so you can do what you need with them. :-)

• Adding a new C file for a single variable feels like overkill, but it didn’t seem to fit in any of the existing ones. Any opinions?

Small C files don't bother me. I prefer them when they match the common structure of everything.

[cwalther]

As a sign of life, here is the previous state rebased on current main. I have not made any of the other promised/planned changes yet.

[hierophect]

@cwalther just a heads up that I'm also working on rebasing/reworking this too (so we don't overlap in case you were considering getting back into it)

[cwalther]

Great, thank you. I was in fact thinking of having a go at it this weekend, but you’re so much faster than I that you better just go ahead. I first want to take a look at what you did in #3454 and test it, so by the time I get to it you’re probably done.

Is it clear what my planned change regarding REPL runs was? I don’t recall the exact code change off the top of my head, but it was about removing the special case with MP_OBJ_SENTINEL.

[hierophect]

Great, thank you. I was in fact thinking of having a go at it this weekend, but you’re so much faster than I that you better just go ahead. I first want to take a look at what you did in #3454 and test it, so by the time I get to it you’re probably done.

I really didn't have to do that much to #3454, it was mostly just checking that the merge didn't break anything, and replacing those gotos with a loop skip. It took longer to just grok how the code needed to maneuver around deep sleep than actually making the changes.

I don't have a strong opinion about the REPL runs. REPL access strikes me as a pitstop you use to check things as opposed to a part of any actual program/debugging flow, so retaining program details across it doesn't bother me conceptually. If someone is testing specific program behavior, having the REPL not reset their flow might be convenient. I'll probably just leave it as it is, we can revisit if people complain that it's clunky or something.

[cwalther]

In case it helps as a point of reference, I have done the rebasing here: cwalther/getprevtraceback2. I haven’t checked very carefully whether the merging is sane (in particular, whether the change to rgb_led_status.c should really be gone without replacement), but it compiles and runs.

[hierophect]

@cwalther I'm sorry I've been taking a while to catch up on this, I've had a chaotic last couple of weeks. I'm hoping to get it fully tested next week.

[hierophect]

Superseding with #5037 to avoid force pushing the rebase.