Skip to content

Parse with errors - #34

Merged
uncomputable merged 7 commits into
BlockstreamResearch:masterfrom
uncomputable:parse-errors
May 13, 2024
Merged

Parse with errors#34
uncomputable merged 7 commits into
BlockstreamResearch:masterfrom
uncomputable:parse-errors

Conversation

@uncomputable

@uncomputable uncomputable commented May 11, 2024

Copy link
Copy Markdown
Collaborator

Add infrastructure for relaying pretty errors to the user. Enable errors during parsing (the compiler still panics). Avoid unwraps.

Format the error messages similar to how the rust compiler does it.

  |
1 | let a1: List<u32, 2> = None;
  |              ^^^^^^ List bound must be a power of two greater one: 2, 4, 8, 16, 32, ...

Support multi-line errors.

  |
2 | let x: u32 = Left(
3 |     Right(0)
4 | );
  | ^^^^^^^^^^^^^^^^^^ Type mismatch: Expected value of type `u32`, got `Either<Either<_, u32>, _>`

An error (RichError) consists of an error variant (Error) and a context (spanned area inside a file). During parsing we attach the line/column position of the affected area. The parser works on local chunks and doesn't have access to the entire file. When parsing is done, we attach the entire file to enable pretty printing. The human encoding follows the same approach.

Supersedes #27

@uncomputable
uncomputable requested a review from apoelstra May 11, 2024 21:01
`(A,B)` -> `(A, B)`
Track the area of objects inside a source file.
`Error` reports a local problem.
`RichError` reports a local problem in a broader context
(file, line, column, start, end).

The file is added to `RichError` last minute, after the parsing,
because it is not locally available during parsing.
Display pretty errors from the PEST parser to the user.
Adapt the compile / satisfy method in lib.rs and the CLI interface.
Display pretty errors from the parse tree generator to the user.
Unwraps added in this commit are inside unit tests only.
Remove duplicate implementation of the compile function.
@apoelstra

Copy link
Copy Markdown
Contributor

In 6bebb3862abbb93027cdbc5c4c5b575de7962c3d:

I think it'd be better to move TypeName into type.rs than to move innards of type.rs into the jet module and then have to expose a ton of private stuff.

@apoelstra

apoelstra commented May 13, 2024

Copy link
Copy Markdown
Contributor

In 5807df7fca538fd6f5e0a0a7f014ba174102c41e:

Why can't inference be a submodule of types. Then it would have access to the private data from types but you won't need to expose it. In particular making UnificationVar have public fields seems like a super bad idea.

@apoelstra

Copy link
Copy Markdown
Contributor

Renames in 576a62cceb580e2dc7a5b3c59fab0956d137e204 looks good

@apoelstra

Copy link
Copy Markdown
Contributor

In 705d2f6768e9a47b120aa6d68986ed888be9b03e:

How does this variable factor relate to the existing one in src/types/variable.rs? It seems like this commit partially reverts #133 which removed the old variable factory, but doesn't make any reference to it. Can you clarify this?

@apoelstra

Copy link
Copy Markdown
Contributor

Ignore all the above comments. I was reviewing #34 in rust-simplicity, which looks weird because it's from 2022 and predates all the stuff that I'm mentioning :P.

@apoelstra

Copy link
Copy Markdown
Contributor

dbd1196 looks good!

One nit is that I see you replaced an assert_eq with a debug_assert ... I am pretty sure there is a debug_assert_eq. But I guess you tried that.

@apoelstra apoelstra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK dbd1196

@uncomputable
uncomputable merged commit d8bc492 into BlockstreamResearch:master May 13, 2024
@uncomputable
uncomputable deleted the parse-errors branch May 13, 2024 20:18
@uncomputable

uncomputable commented May 13, 2024

Copy link
Copy Markdown
Collaborator Author

I actually didn't know about debug_assert_eq. For some reason I thought it wasn't available, maybe because my IDE doesn't know it. It seems to exist since Rust 1.0.0. I will start using it from now.

This was referenced May 13, 2024
uncomputable added a commit that referenced this pull request May 14, 2024
42e2738 Compile (arrays|lists) with errors (Christian Lewe)
23ec5d7 Compile with errors (Christian Lewe)
499d16d Scope: Don't panic upon undefined variables (Christian Lewe)
82759ee Parse decimals with errors (Christian Lewe)
7eabe5d Error: Add compile errors (Christian Lewe)
d5982b2 Parse: Add JetName (Christian Lewe)

Pull request description:

  Extend #34 to include compile errors.

ACKs for top commit:
  apoelstra:
    ACK 42e2738

Tree-SHA512: f4962e14be0be7af00c3b55c225944207c67e5c4f55b8d6a4c78dd37bd2cbdd5c4e6c2122d9920f08f2e8cf3e1274be768be58226e05199cb1d3e75f5ce968ea
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

Successfully merging this pull request may close these issues.

2 participants