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

Improve error message on duplicate non-mergeable fields #229

Merged
merged 1 commit into from May 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 9 additions & 12 deletions dhall/src/operations/typecheck.rs
Expand Up @@ -17,23 +17,20 @@ fn check_rectymerge(
x: Nir<'_>,
y: Nir<'_>,
) -> Result<(), TypeError> {
let not_record_err = || match span {
Span::DuplicateRecordFieldsSugar(_, r) => {
mk_span_err((**r).clone(), "DuplicateFieldName")
}
_ => mk_span_err(span.clone(), "RecordTypeMergeRequiresRecordType"),
};

let kts_x = match x.kind() {
NirKind::RecordType(kts) => kts,
_ => {
return mk_span_err(
span.clone(),
"RecordTypeMergeRequiresRecordType",
)
}
_ => return not_record_err(),
};
let kts_y = match y.kind() {
NirKind::RecordType(kts) => kts,
_ => {
return mk_span_err(
span.clone(),
"RecordTypeMergeRequiresRecordType",
)
}
_ => return not_record_err(),
};
for (k, tx) in kts_x {
if let Some(ty) = kts_y.get(k) {
Expand Down
2 changes: 1 addition & 1 deletion dhall/src/syntax/ast/span.rs
Expand Up @@ -19,7 +19,7 @@ pub enum Span {
/// A location in the source text
Parsed(ParsedSpan),
/// Desugarings
DuplicateRecordFieldsSugar,
DuplicateRecordFieldsSugar(Box<Span>, Box<Span>),
DottedFieldSugar,
RecordPunSugar,
/// For expressions obtained from decoding binary
Expand Down
10 changes: 7 additions & 3 deletions dhall/src/syntax/text/parser.rs
Expand Up @@ -97,9 +97,13 @@ fn insert_recordlit_entry(map: &mut BTreeMap<Label, Expr>, l: Label, e: Expr) {
Entry::Occupied(mut entry) => {
let dummy = Expr::new(Num(Bool(false)), Span::Artificial);
let other = entry.insert(dummy);
let span = Span::DuplicateRecordFieldsSugar(
Box::new(other.span()),
Box::new(e.span()),
);
entry.insert(Expr::new(
Op(BinOp(RecursiveRecordMerge, other, e)),
Span::DuplicateRecordFieldsSugar,
span,
));
}
}
Expand Down Expand Up @@ -133,11 +137,11 @@ lazy_static::lazy_static! {
};
}

// Generate pest parser manually becaue otherwise we'd need to modify something outside of OUT_DIR
// Generate pest parser manually because otherwise we'd need to modify something outside of OUT_DIR
// and that's forbidden by docs.rs.
// This is equivalent to:
// ```
// #[derive(Parser)
// #[derive(Parser)]
// #[grammar = "..."]
// struct DhallParser;
// ```
Expand Down
@@ -1 +1,12 @@
Type error: error: RecordTypeMergeRequiresRecordType
Type error: error: DuplicateFieldName
--> <current file>:6:47
|
1 | {- This test illustrates that duplicate fields need not be literals in order
2 | to be properly normalized. One or both of the duplicate fields can be
3 | abstract because field duplication delegates its behavior to the `∧`
4 | operator
...
12 | -}
13 | λ(r : { y : Natural }) → { x = { y = 1 }, x = r }
| ^ DuplicateFieldName
|
@@ -1 +1,7 @@
Type error: error: RecordTypeMergeRequiresRecordType
Type error: error: DuplicateFieldName
--> <current file>:1:22
|
...
8 | { x = { y = 0 }, x = { y = 0 } }
| ^^^^^^^^^ DuplicateFieldName
|
@@ -1 +1,7 @@
Type error: error: RecordTypeMergeRequiresRecordType
Type error: error: DuplicateFieldName
--> <current file>:1:14
|
...
7 | { x = 0, x = 0 }
| ^ DuplicateFieldName
|