Skip to content

Commit a2b2bc0

Browse files
yannhamyannham
authored
Merge pull request #2359 from tweag/rfc007/typecheck-error-new-ast
Use Ast instead of RichTerm for typechecking error
2 parents 05add69 + d2c8feb commit a2b2bc0

File tree

10 files changed

+581
-307
lines changed

10 files changed

+581
-307
lines changed

‎core/src/error/mod.rs‎

Lines changed: 310 additions & 102 deletions
Large diffs are not rendered by default.

‎core/src/pretty.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ impl Allocator {
559559
match id_expr.as_ref() {
560560
// Nickel will not parse a multiline string literal in this position
561561
Term::StrChunks(chunks) => self.chunks(chunks, StringRenderStyle::ForceMonoline),
562-
Term::ParseError(_) => docs![self, "<parse error>"],
562+
Term::ParseError(_) => docs![self, "%<parse error>"],
563563
_ => unimplemented!("Dynamic record fields must be StrChunks currently"),
564564
}
565565
.append(self.field_body(field))
@@ -1180,8 +1180,8 @@ impl<'a> Pretty<'a, Allocator> for &Term {
11801180
ResolvedImport(id) => allocator.text(format!("import <file_id: {id:?}>")),
11811181
// This type is in term position, so we don't need to add parentheses.
11821182
Type { typ, contract: _ } => typ.pretty(allocator),
1183-
ParseError(_) => allocator.text("%<PARSE ERROR>"),
1184-
RuntimeError(_) => allocator.text("%<RUNTIME ERROR>"),
1183+
ParseError(_) => allocator.text("%<parse error>"),
1184+
RuntimeError(_) => allocator.text("%<runtime error>"),
11851185
Closure(idx) => allocator.text(format!("%<closure@{idx:p}>")),
11861186
}
11871187
}

‎core/src/typ.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,11 @@ impl From<UnboundTypeVariableError> for EvalError {
758758

759759
impl From<UnboundTypeVariableError> for TypecheckError {
760760
fn from(err: UnboundTypeVariableError) -> Self {
761-
TypecheckError::UnboundTypeVariable(err.0)
761+
use crate::{bytecode::ast::alloc::AstAlloc, error::TypecheckErrorData};
762+
763+
TypecheckError::new(AstAlloc::new(), |_alloc| {
764+
TypecheckErrorData::UnboundTypeVariable(err.0)
765+
})
762766
}
763767
}
764768

‎core/src/typecheck/error.rs‎

Lines changed: 179 additions & 122 deletions
Large diffs are not rendered by default.

‎core/src/typecheck/mod.rs‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,12 @@
5656
//! [HasApparentType]).
5757
use crate::{
5858
bytecode::ast::{
59-
alloc::CloneTo, compat::ToMainline, pattern::bindings::Bindings as _, record::FieldDef,
60-
typ::*, Annotation, Ast, AstAlloc, LetBinding, MatchBranch, Node, StringChunk, TryConvert,
59+
alloc::CloneTo, pattern::bindings::Bindings as _, record::FieldDef, typ::*, Annotation,
60+
Ast, AstAlloc, LetBinding, MatchBranch, Node, StringChunk, TryConvert,
6161
},
6262
cache::AstImportResolver,
6363
environment::Environment,
64-
error::TypecheckError,
64+
error::{TypecheckError, TypecheckErrorData},
6565
identifier::{Ident, LocIdent},
6666
mk_uty_arrow, mk_uty_enum, mk_uty_record, mk_uty_record_row,
6767
position::TermPos,
@@ -1275,12 +1275,13 @@ impl<'ast> Context<'ast> {
12751275
}
12761276

12771277
/// Retrieves a variable from the type environment, or fail with
1278-
/// [crate::error::TypecheckError::UnboundIdentifier] instead.
1278+
/// [crate::error::TypecheckErrorData::UnboundIdentifier] instead.
12791279
pub fn get_type(&self, id: LocIdent) -> Result<UnifType<'ast>, TypecheckError> {
1280-
self.type_env
1281-
.get(&id.ident())
1282-
.cloned()
1283-
.ok_or_else(|| TypecheckError::UnboundIdentifier(id))
1280+
self.type_env.get(&id.ident()).cloned().ok_or_else(|| {
1281+
TypecheckError::new(AstAlloc::new(), |_alloc| {
1282+
TypecheckErrorData::UnboundIdentifier(id)
1283+
})
1284+
})
12841285
}
12851286
}
12861287

@@ -2445,10 +2446,12 @@ impl<'ast> Check<'ast> for &'ast Ast<'ast> {
24452446
}
24462447
Node::Type(typ) => {
24472448
if let Some(contract) = typ.find_contract() {
2448-
Err(TypecheckError::CtrTypeInTermPos {
2449-
contract: contract.to_mainline(),
2450-
pos: self.pos,
2451-
})
2449+
Err(TypecheckError::new(AstAlloc::new(), |alloc| {
2450+
TypecheckErrorData::CtrTypeInTermPos {
2451+
contract: Ast::clone_to(contract.clone(), alloc),
2452+
pos: self.pos,
2453+
}
2454+
}))
24522455
} else {
24532456
Ok(())
24542457
}

‎core/src/typecheck/pattern.rs‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use crate::{
22
bytecode::ast::pattern::*,
3-
error::TypecheckError,
43
identifier::{Ident, LocIdent},
54
mk_uty_record_row,
65
typ::{EnumRowsF, RecordRowsF, TypeF},
@@ -608,10 +607,12 @@ impl<'ast> PatternTypes<'ast> for OrPattern<'ast> {
608607
pat_bindings.last().unwrap().0
609608
};
610609

611-
return Err(TypecheckError::OrPatternVarsMismatch {
612-
var: witness,
613-
pos: self.pos,
614-
});
610+
return Err(TypecheckError::new(AstAlloc::new(), |_alloc| {
611+
TypecheckErrorData::OrPatternVarsMismatch {
612+
var: witness,
613+
pos: self.pos,
614+
}
615+
}));
615616
}
616617

617618
// We unify the type of the first or-branch with the current or-branch, to make sure
@@ -632,10 +633,12 @@ impl<'ast> PatternTypes<'ast> for OrPattern<'ast> {
632633
// smaller one, which is guaranteed to be missing (indeed, the greater one
633634
// could still appear later in the other list, but the smaller is necessarily
634635
// missing in the list with the greater one)
635-
return Err(TypecheckError::OrPatternVarsMismatch {
636-
var: std::cmp::min(*model_id, id),
637-
pos: self.pos,
638-
});
636+
return Err(TypecheckError::new(AstAlloc::new(), |_alloc| {
637+
TypecheckErrorData::OrPatternVarsMismatch {
638+
var: std::cmp::min(*model_id, id),
639+
pos: self.pos,
640+
}
641+
}));
639642
}
640643

641644
if let TypecheckMode::Enforce = mode {

‎core/tests/integration/main.rs‎

Lines changed: 53 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::{io::Cursor, thread};
33
use nickel_lang_core::{
44
error::{
55
Error, EvalError, ExportError, ExportErrorData, ImportError, NullReporter, ParseError,
6-
TypecheckError,
6+
TypecheckErrorData,
77
},
88
term::Term,
99
typecheck::TypecheckMode,
@@ -261,23 +261,6 @@ impl PartialEq<Error> for ErrorExpectation {
261261
Error::EvalError(EvalError::NonExhaustiveEnumMatch { .. }),
262262
)
263263
| (EvalFailedDestructuring, Error::EvalError(EvalError::FailedDestructuring { .. }))
264-
| (
265-
TypecheckRecordRowMismatch,
266-
Error::TypecheckError(TypecheckError::RecordRowMismatch { .. }),
267-
)
268-
| (
269-
TypecheckEnumRowMismatch,
270-
Error::TypecheckError(TypecheckError::EnumRowMismatch { .. }),
271-
)
272-
| (
273-
TypecheckMissingDynTail,
274-
Error::TypecheckError(TypecheckError::MissingDynTail { .. }),
275-
)
276-
| (TypecheckExtraDynTail, Error::TypecheckError(TypecheckError::ExtraDynTail { .. }))
277-
| (
278-
TypecheckCtrTypeInTermPos,
279-
Error::TypecheckError(TypecheckError::CtrTypeInTermPos { .. }),
280-
)
281264
| (ImportParseError, Error::ImportError(ImportError::ParseErrors(..)))
282265
| (ImportIoError, Error::ImportError(ImportError::IOError(..)))
283266
| (
@@ -313,92 +296,108 @@ impl PartialEq<Error> for ErrorExpectation {
313296
EvalMissingFieldDef { field },
314297
Error::EvalError(EvalError::MissingFieldDef { id, .. }),
315298
) => field == id.label(),
299+
(_, Error::TypecheckError(tc_err)) => self == tc_err.borrow_error(),
300+
(_, _) => false,
301+
}
302+
}
303+
}
304+
305+
impl PartialEq<TypecheckErrorData<'_>> for ErrorExpectation {
306+
fn eq(&self, other: &TypecheckErrorData) -> bool {
307+
use ErrorExpectation::*;
308+
309+
match (self, other) {
310+
(TypecheckRecordRowMismatch, TypecheckErrorData::RecordRowMismatch { .. })
311+
| (TypecheckEnumRowMismatch, TypecheckErrorData::EnumRowMismatch { .. })
312+
| (TypecheckMissingDynTail, TypecheckErrorData::MissingDynTail { .. })
313+
| (TypecheckExtraDynTail, TypecheckErrorData::ExtraDynTail { .. })
314+
| (TypecheckCtrTypeInTermPos, TypecheckErrorData::CtrTypeInTermPos { .. }) => true,
316315
(
317316
TypecheckUnboundIdentifier { identifier },
318-
Error::TypecheckError(TypecheckError::UnboundIdentifier(id)),
317+
TypecheckErrorData::UnboundIdentifier(id),
319318
) => id.label() == identifier,
320319
(
321320
TypecheckUnboundTypeVariable { identifier },
322-
Error::TypecheckError(TypecheckError::UnboundTypeVariable(ident)),
321+
TypecheckErrorData::UnboundTypeVariable(ident),
323322
) => identifier == ident.label(),
324323
(
325324
TypecheckTypeMismatch { expected, inferred },
326-
Error::TypecheckError(
327-
TypecheckError::TypeMismatch {
328-
expected: expected1,
329-
inferred: inferred1,
330-
..
331-
}
332-
| TypecheckError::ArrowTypeMismatch {
333-
expected: expected1,
334-
inferred: inferred1,
335-
..
336-
},
337-
),
325+
TypecheckErrorData::TypeMismatch {
326+
expected: expected1,
327+
inferred: inferred1,
328+
..
329+
}
330+
| TypecheckErrorData::ArrowTypeMismatch {
331+
expected: expected1,
332+
inferred: inferred1,
333+
..
334+
},
338335
) if expected == &expected1.to_string() && inferred == &inferred1.to_string() => true,
339336
(
340337
TypecheckForallParametricityViolation {
341338
tail,
342339
violating_type,
343340
},
344-
Error::TypecheckError(TypecheckError::ForallParametricityViolation {
341+
TypecheckErrorData::ForallParametricityViolation {
345342
tail: tail1,
346343
violating_type: vtype1,
347344
..
348-
}),
345+
},
349346
) => {
350347
tail.as_str() == tail1.to_string() && violating_type.as_str() == vtype1.to_string()
351348
}
352-
(
353-
TypecheckMissingRow { ident },
354-
Error::TypecheckError(TypecheckError::MissingRow { id: id1, .. }),
355-
) if ident == id1.label() => true,
356-
(
357-
TypecheckExtraRow { ident },
358-
Error::TypecheckError(TypecheckError::ExtraRow { id: id1, .. }),
359-
) if ident == id1.label() => true,
349+
(TypecheckMissingRow { ident }, TypecheckErrorData::MissingRow { id: id1, .. })
350+
if ident == id1.label() =>
351+
{
352+
true
353+
}
354+
(TypecheckExtraRow { ident }, TypecheckErrorData::ExtraRow { id: id1, .. })
355+
if ident == id1.label() =>
356+
{
357+
true
358+
}
360359
(
361360
TypecheckRecordRowConflict { row },
362-
Error::TypecheckError(TypecheckError::RecordRowConflict { row: row1, .. }),
361+
TypecheckErrorData::RecordRowConflict { row: row1, .. },
363362
) => row == row1.id.label(),
364363
(
365364
TypecheckEnumRowConflict { row },
366-
Error::TypecheckError(TypecheckError::EnumRowConflict { row: row1, .. }),
365+
TypecheckErrorData::EnumRowConflict { row: row1, .. },
367366
) => row == row1.id.label(),
368367
(
369368
TypecheckVarLevelMismatch { type_var: ident },
370-
Error::TypecheckError(TypecheckError::VarLevelMismatch {
369+
TypecheckErrorData::VarLevelMismatch {
371370
type_var: constant, ..
372-
}),
371+
},
373372
) => ident == constant.label(),
374373
(
375374
TypecheckInhomogeneousRecord {
376375
row_a: row1,
377376
row_b: row2,
378377
},
379-
Error::TypecheckError(TypecheckError::InhomogeneousRecord { row_a, row_b, .. }),
378+
TypecheckErrorData::InhomogeneousRecord { row_a, row_b, .. },
380379
) => row1 == &row_a.to_string() && row2 == &row_b.to_string(),
381380
(
382381
TypecheckOrPatternVarsMismatch { var },
383-
Error::TypecheckError(TypecheckError::OrPatternVarsMismatch { var: id, .. }),
382+
TypecheckErrorData::OrPatternVarsMismatch { var: id, .. },
384383
) => var == id.label(),
385384
// The clone is not ideal, but currently we can't compare `TypecheckError` directly
386385
// with an ErrorExpectation. Ideally, we would implement `eq` for all error subtypes,
387386
// and have the eq with `Error` just dispatch to those sub-eq functions.
388387
(
389388
TypecheckArrowTypeMismatch { cause },
390-
Error::TypecheckError(TypecheckError::ArrowTypeMismatch { cause: cause2, .. }),
391-
) => cause.as_ref() == &Error::TypecheckError((**cause2).clone()),
389+
TypecheckErrorData::ArrowTypeMismatch { cause: cause2, .. },
390+
) => cause.as_ref() == &**cause2,
392391
// If nothing else matched up to this point, we allow the expected error to appear wrapped inside an `ArrowTypeMismatch`
393392
//
394-
(error_exp, Error::TypecheckError(TypecheckError::ArrowTypeMismatch { cause, .. })) => {
395-
error_exp == &Error::TypecheckError((**cause).clone())
393+
(error_exp, TypecheckErrorData::ArrowTypeMismatch { cause, .. }) => {
394+
error_exp == &**cause
396395
}
397396
// We equate `TypecheckError(ImportError(x))` with `Error::ImportError(x)`
398-
(error_exp, Error::TypecheckError(TypecheckError::ImportError(import_err))) => {
397+
(error_exp, TypecheckErrorData::ImportError(import_err)) => {
399398
error_exp == &Error::ImportError(import_err.clone())
400399
}
401-
(_, _) => false,
400+
_ => false,
402401
}
403402
}
404403
}

‎lsp/nls/src/world.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,11 +197,11 @@ impl World {
197197

198198
// Make a record of I/O errors in imports so that we can retry them when appropriate.
199199
fn associate_failed_import(&mut self, err: &nickel_lang_core::error::TypecheckError) {
200-
if let nickel_lang_core::error::TypecheckError::ImportError(ImportError::IOError(
200+
if let nickel_lang_core::error::TypecheckErrorData::ImportError(ImportError::IOError(
201201
name,
202202
_,
203203
pos,
204-
)) = &err
204+
)) = err.borrow_error()
205205
{
206206
if let Some((filename, pos)) = PathBuf::from(name).file_name().zip(pos.into_opt()) {
207207
self.failed_imports

‎lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__diagnostics-unparseable-type.ncl.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
source: lsp/nls/tests/main.rs
33
expression: output
44
---
5-
[0:10-0:11: "incompatible types\nExpected an expression of type `{ <parse error>, }` (a contract)\nFound an expression of type `{ _ : _ }`\nStatic types and contracts are not compatible", 0:10-0:11: "this expression", 0:32-0:33: "unexpected token"]
5+
[0:10-0:11: "incompatible types\nExpected an expression of type `{ %<parse error> }` (a contract)\nFound an expression of type `{ _ : _ }`\nStatic types and contracts are not compatible", 0:10-0:11: "this expression", 0:32-0:33: "unexpected token"]

‎lsp/nls/tests/snapshots/main__lsp__nls__tests__inputs__no-crash-on-pretty-print.ncl.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
source: lsp/nls/tests/main.rs
33
expression: output
44
---
5-
[2:63-2:64: "unexpected token", 3:6-3:7: "incompatible types\nExpected an expression of type `Number -> Dyn -> [| 'Ok, 'Error { message | String, <parse error>, } |]`\nFound an expression of type `Number`\nThese types are not compatible", 3:6-3:7: "this expression"]
5+
[2:63-2:64: "unexpected token", 3:6-3:7: "incompatible types\nExpected an expression of type `Number -> Dyn -> [| 'Ok, 'Error { message | String, %<parse error> } |]`\nFound an expression of type `Number`\nThese types are not compatible", 3:6-3:7: "this expression"]

0 commit comments

Comments
 (0)