postprocess: Add --on-error flag for non-fatal transform failures

[Crocodoctopus]

Adds configurable transform error handling via --on-error.

Comment transform failures now raise TransformError instead of relying on a hard assert. The CLI can either abort on the first failure, keep going while returning exit 1, or warn and return exit 0.

As part of this, cache.update(...) now happens only after the comment parity check succeeds.

[thedataking]

Right now, the postprocessor only transfers comments but we have PRs to process assertions, and other useful cleanup. I don't think we should have a comment-specific flag.

Please consider a more generic design where we accept a flag (or flag pair --fail-fast/--no-fail-fast with the former being the default) that any Transform that derives from AbstractTransform can use to decide whether to keep going or not.

[Crocodoctopus]

The flag ended up being --continue-on-error, which is now passed to AbstractTransform for future transforms.

[thedataking]

The flag ended up being --continue-on-error, which is now passed to AbstractTransform for future transforms.

cargo test supports --keep-going which is what you initially proposed.
I mentioned --fail-fast, --no-fail-fast because cargo nextest exposes that.
Why did you chose --continue-on-error? Considering all the options, I think your --keep-going suggestion is best since (I assume) more people use and know about the cargo test CLI than the cargo nextest one.

[thedataking]

from offline discussion: --keep-going or --no-fail-fast isn't exactly what we need here since we also want to option to downgrade errors to warnings so postprocessing can run in best effort mode. Simplest solution that covers CI, development, and CRISP use requirements:

Tri-state on-error {abort, keep-going, warn} flag. With the following semantics:

• abort: stop at first failure, nonzero exit (development/debugging)
• keep-going: process everything, report all failures, nonzero exit (CI)
• warn: process everything, log failures as warnings, exit 0 (embedding in a larger tool)

keep-going is the safe default for users of c2rust.

We can use exceptions to simplify error handling:

  class TransformError(Exception): ...

  if c_comments != rust_comments:
      raise TransformError(f"comment mismatch for {identifier}: {c_comments=} {rust_comments=}")

Then the policy logic goes here:

1. The per-function loop in apply_file (base.py:108) wraps the apply_ident call in try/except TransformError; on abort it re-raises, otherwise it records the failure and
   continues.
2. main() looks at the collected failure count at the end: nonzero exit unless mode is warn (where it logs a summary like "postprocessed N functions, skipped M due to errors" and returns 0).

For PR #1849 specifically (not blocking for this PR) :
One complication is that we might want to always keep going if TrimTransform fails but the on-error flag is flexible enough to allow more states if we absolutely need them.

[thedataking]

PR body is stale. Cache.update was moved to after the comment-verbatim check. Worth doing but you should document the change when you update the PR body.

continue_on_error appears to be a vestige from previous iteration; remove it?

[Crocodoctopus]

I was unaware the top-most comment should be edited to reflect the current state of the PR, I'll do that from now on.

Instances of the old continue_on_error were all removed.

This should be g2g?

[thedataking]

LGTM with one nitpick:

Please undo formatting changes in transforms/init.py and comments.py.

• transforms/__init__.py changes can just be removed from this commit entirely
• comments.py: just need to revert CommentsTransform.__init__

(logging.exception works well here, nice find. note that it prints the error at the end of the traceback as well as in the f-string you pass as an argument - should be harmless.)