From b68c3af578d1f6b0d1e32e7d88ef57774fb468d8 Mon Sep 17 00:00:00 2001
From: Nadrieril
Date: Mon, 11 Nov 2019 11:20:32 +0000
Subject: Capture absence of span in Span itself

---
 dhall/src/phase/binary.rs    | 10 ++++++++--
 dhall/src/phase/typecheck.rs |  6 +++++-
 2 files changed, 13 insertions(+), 3 deletions(-)

(limited to 'dhall/src/phase')

diff --git a/dhall/src/phase/binary.rs b/dhall/src/phase/binary.rs
index 8c98f77..b4f18da 100644
--- a/dhall/src/phase/binary.rs
+++ b/dhall/src/phase/binary.rs
@@ -5,8 +5,9 @@ use std::vec;
 
 use dhall_syntax::map::DupTreeMap;
 use dhall_syntax::{
-    rc, Expr, ExprF, FilePath, FilePrefix, Hash, Import, ImportLocation,
-    ImportMode, Integer, InterpolatedText, Label, Natural, Scheme, URL, V,
+    Expr, ExprF, FilePath, FilePrefix, Hash, Import, ImportLocation,
+    ImportMode, Integer, InterpolatedText, Label, Natural, RawExpr, Scheme,
+    Span, URL, V,
 };
 
 use crate::error::{DecodeError, EncodeError};
@@ -24,6 +25,11 @@ pub(crate) fn encode<E>(expr: &Expr<E>) -> Result<Vec<u8>, EncodeError> {
         .map_err(|e| EncodeError::CBORError(e))
 }
 
+// Should probably rename this
+pub fn rc<E>(x: RawExpr<E>) -> Expr<E> {
+    Expr::new(x, Span::Decoded)
+}
+
 fn cbor_value_to_dhall(data: &cbor::Value) -> Result<DecodedExpr, DecodeError> {
     use cbor::Value::*;
     use dhall_syntax::{BinOp, Builtin, Const};
diff --git a/dhall/src/phase/typecheck.rs b/dhall/src/phase/typecheck.rs
index 9013c1f..4d8c7d3 100644
--- a/dhall/src/phase/typecheck.rs
+++ b/dhall/src/phase/typecheck.rs
@@ -2,7 +2,7 @@ use std::cmp::max;
 use std::collections::HashMap;
 
 use dhall_syntax::{
-    rc, Builtin, Const, Expr, ExprF, InterpolatedTextContents, Label,
+    Builtin, Const, Expr, ExprF, InterpolatedTextContents, Label, RawExpr, Span,
 };
 
 use crate::core::context::TypecheckContext;
@@ -142,6 +142,10 @@ pub(crate) fn const_to_value(c: Const) -> Value {
     }
 }
 
+pub fn rc<E>(x: RawExpr<E>) -> Expr<E> {
+    Expr::new(x, Span::Artificial)
+}
+
 // Ad-hoc macro to help construct the types of builtins
 macro_rules! make_type {
     (Type) => { ExprF::Const(Const::Type) };
-- 
cgit v1.3.1


From 5d5a356b8eb36e277c312e5550d1cb0a2f82e9fa Mon Sep 17 00:00:00 2001
From: Nadrieril
Date: Mon, 11 Nov 2019 11:34:29 +0000
Subject: Store a `Span` in `Value`

---
 dhall/src/core/value.rs       | 23 +++++++++++++++++++----
 dhall/src/phase/typecheck.rs  | 14 ++++++++++----
 dhall_syntax/src/core/span.rs |  3 +++
 3 files changed, 32 insertions(+), 8 deletions(-)

(limited to 'dhall/src/phase')

diff --git a/dhall/src/core/value.rs b/dhall/src/core/value.rs
index b4b6b08..b6c156e 100644
--- a/dhall/src/core/value.rs
+++ b/dhall/src/core/value.rs
@@ -1,7 +1,7 @@
 use std::cell::{Ref, RefCell, RefMut};
 use std::rc::Rc;
 
-use dhall_syntax::{Builtin, Const};
+use dhall_syntax::{Builtin, Const, Span};
 
 use crate::core::context::TypecheckContext;
 use crate::core::valuef::ValueF;
@@ -36,6 +36,7 @@ struct ValueInternal {
     value: ValueF,
     /// This is None if and only if `value` is `Sort` (which doesn't have a type)
     ty: Option<Value>,
+    span: Span,
 }
 
 /// Stores a possibly unevaluated value. Gets (partially) normalized on-demand,
@@ -69,18 +70,21 @@ impl ValueInternal {
                 form: Unevaled,
                 value: ValueF::Const(Const::Type),
                 ty: None,
+                span: Span::Artificial,
             },
             |vint| match (&vint.form, &vint.ty) {
                 (Unevaled, Some(ty)) => ValueInternal {
                     form: WHNF,
                     value: normalize_whnf(vint.value, &ty),
                     ty: vint.ty,
+                    span: vint.span,
                 },
                 // `value` is `Sort`
                 (Unevaled, None) => ValueInternal {
                     form: NF,
                     value: ValueF::Const(Const::Sort),
                     ty: None,
+                    span: vint.span,
                 },
                 // Already in WHNF
                 (WHNF, _) | (NF, _) => vint,
@@ -113,11 +117,12 @@ impl ValueInternal {
 }
 
 impl Value {
-    fn new(value: ValueF, form: Form, ty: Value) -> Value {
+    fn new(value: ValueF, form: Form, ty: Value, span: Span) -> Value {
         ValueInternal {
             form,
             value,
             ty: Some(ty),
+            span,
         }
         .into_value()
     }
@@ -126,14 +131,22 @@ impl Value {
             form: NF,
             value: ValueF::Const(Const::Sort),
             ty: None,
+            span: Span::Artificial,
         }
         .into_value()
     }
     pub(crate) fn from_valuef_and_type(v: ValueF, t: Value) -> Value {
-        Value::new(v, Unevaled, t)
+        Value::new(v, Unevaled, t, Span::PlaceHolder)
+    }
+    pub(crate) fn from_valuef_and_type_and_span(
+        v: ValueF,
+        t: Value,
+        span: Span,
+    ) -> Value {
+        Value::new(v, Unevaled, t, span)
     }
     pub(crate) fn from_valuef_and_type_whnf(v: ValueF, t: Value) -> Value {
-        Value::new(v, WHNF, t)
+        Value::new(v, WHNF, t, Span::PlaceHolder)
     }
     pub(crate) fn from_const(c: Const) -> Self {
         const_to_value(c)
@@ -258,6 +271,7 @@ impl Shift for ValueInternal {
             form: self.form,
             value: self.value.shift(delta, var)?,
             ty: self.ty.shift(delta, var)?,
+            span: self.span.clone(),
         })
     }
 }
@@ -285,6 +299,7 @@ impl Subst<Value> for ValueInternal {
             form: Unevaled,
             value: self.value.subst_shift(var, val),
             ty: self.ty.subst_shift(var, val),
+            span: self.span.clone(),
         }
     }
 }
diff --git a/dhall/src/phase/typecheck.rs b/dhall/src/phase/typecheck.rs
index 4d8c7d3..63f5157 100644
--- a/dhall/src/phase/typecheck.rs
+++ b/dhall/src/phase/typecheck.rs
@@ -304,6 +304,7 @@ fn type_with(
     e: Expr<Normalized>,
 ) -> Result<Value, TypeError> {
     use dhall_syntax::ExprF::{Annot, Embed, Lam, Let, Pi, Var};
+    let span = e.span();
 
     Ok(match e.as_ref() {
         Lam(var, annot, body) => {
@@ -348,7 +349,7 @@ fn type_with(
                 |e| type_with(ctx, e.clone()),
                 |_, _| unreachable!(),
             )?;
-            type_last_layer(ctx, expr)?
+            type_last_layer(ctx, expr, span)?
         }
     })
 }
@@ -358,6 +359,7 @@ fn type_with(
 fn type_last_layer(
     ctx: &TypecheckContext,
     e: ExprF<Value, Normalized>,
+    span: Span,
 ) -> Result<Value, TypeError> {
     use crate::error::TypeMessage::*;
     use dhall_syntax::BinOp::*;
@@ -584,6 +586,7 @@ fn type_last_layer(
                 l.get_type()?,
                 r.get_type()?,
             ),
+            Span::PlaceHolder,
         )?),
         BinOp(RecursiveRecordTypeMerge, l, r) => {
             use crate::phase::normalize::merge_maps;
@@ -619,6 +622,7 @@ fn type_last_layer(
                             l.clone(),
                             r.clone(),
                         ),
+                        Span::PlaceHolder,
                     )
                 },
             )?;
@@ -786,9 +790,11 @@ fn type_last_layer(
     };
 
     Ok(match ret {
-        RetTypeOnly(typ) => {
-            Value::from_valuef_and_type(ValueF::PartialExpr(e), typ)
-        }
+        RetTypeOnly(typ) => Value::from_valuef_and_type_and_span(
+            ValueF::PartialExpr(e),
+            typ,
+            span,
+        ),
         RetWhole(v) => v,
     })
 }
diff --git a/dhall_syntax/src/core/span.rs b/dhall_syntax/src/core/span.rs
index fc8de6e..fa89c30 100644
--- a/dhall_syntax/src/core/span.rs
+++ b/dhall_syntax/src/core/span.rs
@@ -22,6 +22,9 @@ pub enum Span {
     Decoded,
     /// For expressions constructed during normalization/typecheck
     Artificial,
+    /// For when there should be a span but it's not done yet
+    /// TODO: properly handle spans
+    PlaceHolder,
 }
 
 impl Span {
-- 
cgit v1.3.1


From 330f063e80a51f8f399864f9d01412e1bff34fe9 Mon Sep 17 00:00:00 2001
From: Nadrieril
Date: Mon, 11 Nov 2019 12:43:09 +0000
Subject: Display first pretty type error

---
 dhall/src/error/mod.rs                             |  8 +++----
 dhall/src/phase/typecheck.rs                       |  2 +-
 dhall/src/tests.rs                                 | 25 +++++++++++++++++++++-
 dhall/tests/type-errors/unit/AssertAlphaTrap.txt   |  7 +++++-
 .../unit/UnionDeprecatedConstructorsKeyword.txt    |  7 +++++-
 dhall/tests/type-errors/unit/VariableFree.txt      |  7 +++++-
 dhall_syntax/src/core/span.rs                      | 14 ++++++++++++
 7 files changed, 61 insertions(+), 9 deletions(-)

(limited to 'dhall/src/phase')

diff --git a/dhall/src/error/mod.rs b/dhall/src/error/mod.rs
index 5338e2d..efbd578 100644
--- a/dhall/src/error/mod.rs
+++ b/dhall/src/error/mod.rs
@@ -1,6 +1,6 @@
 use std::io::Error as IOError;
 
-use dhall_syntax::{BinOp, Import, Label, ParseError, V};
+use dhall_syntax::{BinOp, Import, Label, ParseError, Span};
 
 use crate::core::context::TypecheckContext;
 use crate::core::value::Value;
@@ -48,7 +48,7 @@ pub struct TypeError {
 /// The specific type error
 #[derive(Debug)]
 pub(crate) enum TypeMessage {
-    UnboundVariable(V<Label>),
+    UnboundVariable(Span),
     InvalidInputType(Value),
     InvalidOutputType(Value),
     NotAFunction(Value),
@@ -102,8 +102,8 @@ impl TypeError {
 impl std::fmt::Display for TypeError {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
         use TypeMessage::*;
-        let msg = match self.message {
-            UnboundVariable(_) => "Unbound variable".to_string(),
+        let msg = match &self.message {
+            UnboundVariable(span) => span.error("Unbound variable"),
             InvalidInputType(_) => "Invalid function input".to_string(),
             InvalidOutputType(_) => "Invalid function output".to_string(),
             NotAFunction(_) => "Not a function".to_string(),
diff --git a/dhall/src/phase/typecheck.rs b/dhall/src/phase/typecheck.rs
index 63f5157..9d2c69b 100644
--- a/dhall/src/phase/typecheck.rs
+++ b/dhall/src/phase/typecheck.rs
@@ -339,7 +339,7 @@ fn type_with(
             None => {
                 return Err(TypeError::new(
                     ctx,
-                    TypeMessage::UnboundVariable(var.clone()),
+                    TypeMessage::UnboundVariable(span),
                 ))
             }
         },
diff --git a/dhall/src/tests.rs b/dhall/src/tests.rs
index 9790b95..cde3b03 100644
--- a/dhall/src/tests.rs
+++ b/dhall/src/tests.rs
@@ -20,6 +20,29 @@ right: `{}`"#,
     }};
 }
 
+/// Wrapper around string slice that makes debug output `{:?}` to print string same way as `{}`.
+/// Used in different `assert*!` macros in combination with `pretty_assertions` crate to make
+/// test failures to show nice diffs.
+#[derive(PartialEq, Eq)]
+#[doc(hidden)]
+pub struct PrettyString(String);
+
+/// Make diff to display string as multi-line string
+impl std::fmt::Debug for PrettyString {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        f.write_str(&self.0)
+    }
+}
+
+macro_rules! assert_eq_pretty_str {
+    ($left:expr, $right:expr) => {
+        assert_eq_pretty!(
+            PrettyString($left.to_string()),
+            PrettyString($right.to_string())
+        );
+    };
+}
+
 use std::fs::File;
 use std::io::{Read, Write};
 use std::path::PathBuf;
@@ -172,7 +195,7 @@ pub fn run_test(test: Test<'_>) -> Result<()> {
             if error_file_path.is_file() {
                 let expected_msg = std::fs::read_to_string(error_file_path)?;
                 let msg = format!("{}\n", err);
-                assert_eq_pretty!(msg, expected_msg);
+                assert_eq_pretty_str!(msg, expected_msg);
             } else {
                 std::fs::create_dir_all(error_file_path.parent().unwrap())?;
                 let mut file = File::create(error_file_path)?;
diff --git a/dhall/tests/type-errors/unit/AssertAlphaTrap.txt b/dhall/tests/type-errors/unit/AssertAlphaTrap.txt
index df2b6c1..f2fc8ce 100644
--- a/dhall/tests/type-errors/unit/AssertAlphaTrap.txt
+++ b/dhall/tests/type-errors/unit/AssertAlphaTrap.txt
@@ -1 +1,6 @@
-Type error: Unbound variable
+Type error:  --> 1:47
+  |
+1 | assert : (\(_: Bool) -> _) === (\(x: Bool) -> _)␊
+  |                                               ^
+  |
+  = Unbound variable
diff --git a/dhall/tests/type-errors/unit/UnionDeprecatedConstructorsKeyword.txt b/dhall/tests/type-errors/unit/UnionDeprecatedConstructorsKeyword.txt
index df2b6c1..f7903de 100644
--- a/dhall/tests/type-errors/unit/UnionDeprecatedConstructorsKeyword.txt
+++ b/dhall/tests/type-errors/unit/UnionDeprecatedConstructorsKeyword.txt
@@ -1 +1,6 @@
-Type error: Unbound variable
+Type error:  --> 1:1
+  |
+1 | constructors < Left : Natural | Right : Bool >␊
+  | ^----------^
+  |
+  = Unbound variable
diff --git a/dhall/tests/type-errors/unit/VariableFree.txt b/dhall/tests/type-errors/unit/VariableFree.txt
index df2b6c1..ef1d16e 100644
--- a/dhall/tests/type-errors/unit/VariableFree.txt
+++ b/dhall/tests/type-errors/unit/VariableFree.txt
@@ -1 +1,6 @@
-Type error: Unbound variable
+Type error:  --> 1:1
+  |
+1 | x␊
+  | ^
+  |
+  = Unbound variable
diff --git a/dhall_syntax/src/core/span.rs b/dhall_syntax/src/core/span.rs
index fa89c30..e2bf26d 100644
--- a/dhall_syntax/src/core/span.rs
+++ b/dhall_syntax/src/core/span.rs
@@ -53,4 +53,18 @@ impl Span {
             ),
         }
     }
+
+    pub fn error(&self, message: impl Into<String>) -> String {
+        use pest::error::{Error, ErrorVariant};
+        use pest::Span;
+        let message: String = message.into();
+        let span = match self {
+            self::Span::Parsed(span) => span,
+            _ => return format!("Unknown location: {}", message),
+        };
+        let span = Span::new(&*span.input, span.start, span.end).unwrap();
+        let err: ErrorVariant<!> = ErrorVariant::CustomError { message };
+        let err = Error::new_from_span(err, span);
+        format!("{}", err)
+    }
 }
-- 
cgit v1.3.1


From d28d114552e6c6cb913dce48893fa87e87bf11e2 Mon Sep 17 00:00:00 2001
From: Nadrieril
Date: Mon, 11 Nov 2019 13:36:30 +0000
Subject: Propagate more spans

---
 dhall/src/core/value.rs       | 8 ++++++--
 dhall/src/phase/typecheck.rs  | 6 +++---
 dhall_syntax/src/core/span.rs | 3 ---
 3 files changed, 9 insertions(+), 8 deletions(-)

(limited to 'dhall/src/phase')

diff --git a/dhall/src/core/value.rs b/dhall/src/core/value.rs
index b6c156e..61c7015 100644
--- a/dhall/src/core/value.rs
+++ b/dhall/src/core/value.rs
@@ -136,7 +136,7 @@ impl Value {
         .into_value()
     }
     pub(crate) fn from_valuef_and_type(v: ValueF, t: Value) -> Value {
-        Value::new(v, Unevaled, t, Span::PlaceHolder)
+        Value::new(v, Unevaled, t, Span::Artificial)
     }
     pub(crate) fn from_valuef_and_type_and_span(
         v: ValueF,
@@ -146,7 +146,7 @@ impl Value {
         Value::new(v, Unevaled, t, span)
     }
     pub(crate) fn from_valuef_and_type_whnf(v: ValueF, t: Value) -> Value {
-        Value::new(v, WHNF, t, Span::PlaceHolder)
+        Value::new(v, WHNF, t, Span::Artificial)
     }
     pub(crate) fn from_const(c: Const) -> Self {
         const_to_value(c)
@@ -154,6 +154,10 @@ impl Value {
     pub(crate) fn from_builtin(b: Builtin) -> Self {
         builtin_to_value(b)
     }
+    pub(crate) fn with_span(self, span: Span) -> Self {
+        self.as_internal_mut().span = span;
+        self
+    }
 
     pub(crate) fn as_const(&self) -> Option<Const> {
         match &*self.as_whnf() {
diff --git a/dhall/src/phase/typecheck.rs b/dhall/src/phase/typecheck.rs
index 9d2c69b..33919e4 100644
--- a/dhall/src/phase/typecheck.rs
+++ b/dhall/src/phase/typecheck.rs
@@ -586,7 +586,7 @@ fn type_last_layer(
                 l.get_type()?,
                 r.get_type()?,
             ),
-            Span::PlaceHolder,
+            Span::Artificial,
         )?),
         BinOp(RecursiveRecordTypeMerge, l, r) => {
             use crate::phase::normalize::merge_maps;
@@ -622,7 +622,7 @@ fn type_last_layer(
                             l.clone(),
                             r.clone(),
                         ),
-                        Span::PlaceHolder,
+                        Span::Artificial,
                     )
                 },
             )?;
@@ -795,7 +795,7 @@ fn type_last_layer(
             typ,
             span,
         ),
-        RetWhole(v) => v,
+        RetWhole(v) => v.with_span(span),
     })
 }
 
diff --git a/dhall_syntax/src/core/span.rs b/dhall_syntax/src/core/span.rs
index e2bf26d..d10a53d 100644
--- a/dhall_syntax/src/core/span.rs
+++ b/dhall_syntax/src/core/span.rs
@@ -22,9 +22,6 @@ pub enum Span {
     Decoded,
     /// For expressions constructed during normalization/typecheck
     Artificial,
-    /// For when there should be a span but it's not done yet
-    /// TODO: properly handle spans
-    PlaceHolder,
 }
 
 impl Span {
-- 
cgit v1.3.1