Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Top level error handling #121206

Merged
merged 9 commits into from
Feb 22, 2024
Merged

Commits on Feb 21, 2024

  1. Configuration menu
    Copy the full SHA
    1e16927 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    203b433 View commit details
    Browse the repository at this point in the history
  3. Remove EarlyDiagCtxt::abort_if_errors.

    Its one use isn't necessary, because it's not possible for errors to
    have been emitted at that point.
    nnethercote committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    9919c3d View commit details
    Browse the repository at this point in the history
  4. Adjust the has_errors* methods.

    Currently `has_errors` excludes lint errors. This commit changes it to
    include lint errors.
    
    The motivation for this is that for most places it doesn't matter
    whether lint errors are included or not. But there are multiple places
    where they must be includes, and only one place where they must not be
    included. So it makes sense for `has_errors` to do the thing that fits
    the most situations, and the new `has_errors_excluding_lint_errors`
    method in the one exceptional place.
    
    The same change is made for `err_count`. Annoyingly, this requires the
    introduction of `err_count_excluding_lint_errs` for one place, to
    preserve existing error printing behaviour. But I still think the change
    is worthwhile overall.
    nnethercote committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    46f4983 View commit details
    Browse the repository at this point in the history
  5. Overhaul the handling of errors at the top-level.

    Currently `emit_stashed_diagnostic` is called from four(!) different
    places: `print_error_count`, `DiagCtxtInner::drop`, `abort_if_errors`,
    and `compile_status`.
    
    And `flush_delayed` is called from two different places:
    `DiagCtxtInner::drop` and `Queries`.
    
    This is pretty gross! Each one should really be called from a single
    place, but there's a bunch of entanglements. This commit cleans up this
    mess.
    
    Specifically, it:
    - Removes all the existing calls to `emit_stashed_diagnostic`, and adds
      a single new call in `finish_diagnostics`.
    - Removes the early `flush_delayed` call in `codegen_and_build_linker`,
      replacing it with a simple early return if delayed bugs are present.
    - Changes `DiagCtxtInner::drop` and `DiagCtxtInner::flush_delayed` so
      they both assert that the stashed diagnostics are empty (i.e.
      processed beforehand).
    - Changes `interface::run_compiler` so that any errors emitted during
      `finish_diagnostics` (i.e. late-emitted stashed diagnostics) are
      counted and cannot be overlooked. This requires adding
      `ErrorGuaranteed` return values to several functions.
    - Removes the `stashed_err_count` call in `analysis`. This is possible
      now that we don't have to worry about calling `flush_delayed` early
      from `codegen_and_build_linker` when stashed diagnostics are pending.
    - Changes the `span_bug` case in `handle_tuple_field_pattern_match` to a
      `delayed_span_bug`, because it now can be reached due to the removal
      of the `stashed_err_count` call in `analysis`.
    - Slightly changes the expected output of three tests. If no errors are
      emitted but there are delayed bugs, the error count is no longer
      printed. This is because delayed bugs are now always printed after the
      error count is printed (or not printed, if the error count is zero).
    
    There is a lot going on in this commit. It's hard to break into smaller
    pieces because the existing code is very tangled. It took me a long time
    and a lot of effort to understand how the different pieces interact, and
    I think the new code is a lot simpler and easier to understand.
    nnethercote committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    72b172b View commit details
    Browse the repository at this point in the history
  6. Inline and remove Session::compile_status.

    Because it's now simple enough that it doesn't provide much benefit.
    nnethercote committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    c2512a1 View commit details
    Browse the repository at this point in the history
  7. Refactor run_global_ctxt.

    It currently is infallible and uses `abort_if_errors` and
    `FatalError.raise()` to signal errors. It's easy to instead return a
    `Result<_, ErrorGuaranteed>`, which is the more usual way of doing
    things.
    nnethercote committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    4400644 View commit details
    Browse the repository at this point in the history
  8. Replace unnecessary abort_if_errors.

    Replace `abort_if_errors` calls that are certain to abort -- because
    we emit an error immediately beforehand -- with `FatalErro.raise()`.
    nnethercote committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    4da67ff View commit details
    Browse the repository at this point in the history
  9. Inline and remove abort_on_err.

    It's clumsy and doesn't improve readability.
    nnethercote committed Feb 21, 2024
    Configuration menu
    Copy the full SHA
    f16c226 View commit details
    Browse the repository at this point in the history