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

Document and discuss generator approach to errors #1581

Open
verytactical opened this issue Jan 27, 2025 · 0 comments
Open

Document and discuss generator approach to errors #1581

verytactical opened this issue Jan 27, 2025 · 0 comments

Comments

@verytactical
Copy link
Contributor

verytactical commented Jan 27, 2025

While generators and async generators are quite syntax-heavy, it turned out alternative approaches to logging several errors are even worse.

The approach is:

  • (otherwise uncaught) internal errors unwind the stack in a regular way and are caught at top-level of CLI
  • if compiler found a bug in itself, it throws TactInternalCompilerError, that is also caught at top-level; we don't need a loc?: SrcInfo in it anymore, as it will be handled by step() (see below)
  • if compilation can continue at least for a while after the error, it's yielded as a simple object (for example, every error in parser semantics should be yielded)
    • if the error is intended to be caught and rehandled, the new object type with its own kind has to be defined
    • otherwise an object of predefined types LogDebug, LogInfo or LogError should be yielded; LogError is equivalent to TactCompilationError
  • there is a function step(stage: string, gen: () => AsyncGenerator<...>) that add the name of the step to all yielded Log* texts; it's important to have it working on async generators, so that errors are yielded as soon as they come; there is no way to display these errors at the same time, as they might be produced by several parallel running threads (for example, in resolveImports)
  • function step also catches errors, wraps them into ContextError(child: Error, step: string); if the error is not an instance of Error it wraps it into an UnknownError prior to wrapping
  • there is a function stopOnError(() => AsyncGenerator<...>) that throws an InterruptError if child generator did at least one yield LogError(). In this way we can prevent compiler from continuing into the next compilation stage with invalid state created by error recovery.
  • at top level take all Log* and print them to console.* with runLog(gen: () => AsyncGenerator<...>), doing process.exit(30) if there was a LogError or thrown error, and with filters to verbosity
  • for LSP use a different function that tells IDE when there are LogErrors
  • for external tooling probably supply another function that just collects all Log*s into an array

As the result of this rewrite

  • only throwTactCompilationError and yield Log* are left as the ways to emit errors, new Error() is banned with eslint
  • every compiler stage can produce multiple errors
  • several asynchronous compilations do not interfere with each other
  • there is no place in the entire codebase with console.*, and it's banned with eslint

As further improvement of this approach: pass to Log* functions some initially encoded DSL (currently finally encoded in src/error/display.ts) that has

  • Text(parts: string[], substs) for texts
  • Source that stores code, path and origin
  • when VFS constructor is called, it returns a tuple of VFS itself and unboxSource functions
  • Source can be requested from VFS by any stage that has a reference to it, but it's opaque and can't be unboxed
  • it can be yielded inside of an error message
  • at top level a handler catches all Sources and calls unboxSource to display them
  • SourceCite(path: Source, interval: Interval) to display locations in code with context (the same way as it was ported from ohm)
  • SourceRef(path: Source, interval: Interval) to display identifiers that link to source code locations
  • Interval(from: number, to: number) that get converted to SourceRef when they pass through doParse in parser

and use colored output when interpreting this DSL at top-level of CLI.

As even further improvement, we might opt force Text to only take branded I18n strings that are produced from ICU Message Format files.

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

No branches or pull requests

1 participant