fix: Provide proper line and source information for macro errors

Cargo.toml

@@ -140,6 +140,8 @@ name = "stack_overflow"
 name = "tutorial"
 [[test]]
 name = "vm"
+[[test]]
+name = "ui"
 
 [package.metadata.docs.rs]
 features = ["docs_rs"]

base/src/error.rs

@@ -17,6 +17,12 @@ pub struct Errors<T> {
     errors: Vec<T>,
 }
 
+impl<T> Default for Errors<T> {
+    fn default() -> Self {
+        Errors::new()
+    }
+}
+
 impl<T> Errors<T> {
     /// Creates a new, empty `Errors` instance.
     pub fn new() -> Errors<T> {

benches/check.rs

@@ -1,7 +1,6 @@
 #[macro_use]
 extern crate bencher;
 
-
 extern crate gluon;
 extern crate gluon_base as base;
 extern crate gluon_check as check;
@@ -24,7 +23,7 @@ fn typecheck_prelude(b: &mut Bencher) {
             .unwrap()
             .read_to_string(&mut text)
             .unwrap();
-        text.expand_macro(&mut compiler, &vm, "std.prelude")
+        text.expand_macro(&mut compiler, &vm, "std.prelude", &text)
             .unwrap_or_else(|(_, err)| panic!("{}", err))
     };
     b.iter(|| {

src/compiler_pipeline.rs

@@ -8,6 +8,7 @@
 //! difficult to forget a stage.
 
 use std::borrow::{Borrow, BorrowMut};
+use std::mem;
 use std::ops::Deref;
 use std::result::Result as StdResult;
 
@@ -61,14 +62,15 @@ pub trait MacroExpandable {
         compiler: &mut Compiler,
         thread: &Thread,
         file: &str,
+        expr_str: &str,
     ) -> SalvageResult<MacroValue<Self::Expr>>
     where
         Self: Sized,
     {
         let mut macros = MacroExpander::new(thread);
-        let expr = self.expand_macro_with(compiler, &mut macros, file)?;
+        let expr = self.expand_macro_with(compiler, &mut macros, file, expr_str)?;
         if let Err(err) = macros.finish() {
-            return Err((Some(expr), err.into()));
+            return Err((Some(expr), InFile::new(file, expr_str, err).into()));
         }
         Ok(expr)
     }
@@ -78,6 +80,7 @@ pub trait MacroExpandable {
         compiler: &mut Compiler,
         macros: &mut MacroExpander,
         file: &str,
+        expr_str: &str,
     ) -> SalvageResult<MacroValue<Self::Expr>>;
 }
 
@@ -89,13 +92,14 @@ impl<'s> MacroExpandable for &'s str {
         compiler: &mut Compiler,
         macros: &mut MacroExpander,
         file: &str,
+        expr_str: &str,
     ) -> SalvageResult<MacroValue<Self::Expr>> {
         compiler
             .parse_expr(macros.vm.global_env().type_cache(), file, self)
             .map_err(|err| (None, err.into()))
             .and_then(|mut expr| {
                 let result = (&mut expr)
-                    .expand_macro_with(compiler, macros, file)
+                    .expand_macro_with(compiler, macros, file, expr_str)
                     .map(|_| ())
                     .map_err(|(value, err)| (value.map(|_| ()), err));
                 if let Err((value, err)) = result {
@@ -114,6 +118,7 @@ impl<'s> MacroExpandable for &'s mut SpannedExpr<Symbol> {
         compiler: &mut Compiler,
         macros: &mut MacroExpander,
         file: &str,
+        _expr_str: &str,
     ) -> SalvageResult<MacroValue<Self::Expr>> {
         if compiler.implicit_prelude {
             compiler.include_implicit_prelude(macros.vm.global_env().type_cache(), file, self);
@@ -131,12 +136,19 @@ impl MacroExpandable for SpannedExpr<Symbol> {
         compiler: &mut Compiler,
         macros: &mut MacroExpander,
         file: &str,
+        expr_str: &str,
     ) -> SalvageResult<MacroValue<Self::Expr>> {
         if compiler.implicit_prelude {
             compiler.include_implicit_prelude(macros.vm.global_env().type_cache(), file, &mut self);
         }
+        let prev_errors = mem::replace(&mut macros.errors, Errors::new());
         macros.run(&mut self);
-        Ok(MacroValue { expr: self })
+        let errors = mem::replace(&mut macros.errors, prev_errors);
+        if errors.has_errors() {
+            Err((None, InFile::new(file, expr_str, errors).into()))
+        } else {
+            Ok(MacroValue { expr: self })
+        }
     }
 }
 
@@ -183,7 +195,7 @@ where
         expected_type: Option<&ArcType>,
     ) -> Result<TypecheckValue<Self::Expr>> {
         let mut macro_error = None;
-        let expr = match self.expand_macro(compiler, thread, file) {
+        let expr = match self.expand_macro(compiler, thread, file, expr_str) {
             Ok(expr) => expr,
             Err((Some(expr), err)) => {
                 macro_error = Some(err);

src/import.rs

@@ -4,13 +4,15 @@ use std::any::Any;
 use std::borrow::Cow;
 use std::sync::{Arc, Mutex, RwLock};
 use std::fs::File;
+use std::mem;
 use std::io;
 use std::io::Read;
 use std::path::PathBuf;
 
 use itertools::Itertools;
 
 use base::filename_to_module;
+use base::error::{Errors, InFile};
 use base::ast::{expr_to_path, Expr, Literal, SpannedExpr, Typed, TypedIdent};
 use base::fnv::FnvMap;
 use base::pos::{self, BytePos, Span};
@@ -311,22 +313,37 @@ impl<I> Import<I> {
                 let implicit_prelude = !file_contents.starts_with("//@NO-IMPLICIT-PRELUDE");
                 compiler.set_implicit_prelude(implicit_prelude);
 
-                let errors_before = macros.errors.len();
-                let macro_result =
-                    match file_contents.expand_macro_with(compiler, macros, &modulename) {
-                        Ok(m) => m,
-                        Err((None, err)) => return Err((None, err.into())),
-                        Err((Some(m), err)) => {
-                            macros.errors.push(pos::spanned(span, err.into()));
-                            m
-                        }
-                    };
+                let mut prev_errors = mem::replace(&mut macros.errors, Errors::new());
+
+                let result =
+                    file_contents.expand_macro_with(compiler, macros, &modulename, &file_contents);
+
+                let has_errors = macros.errors.has_errors();
+                let errors = mem::replace(&mut macros.errors, prev_errors);
+                if has_errors {
+                    macros.errors.push(pos::spanned(
+                        span,
+                        Box::new(::Error::Macro(InFile::new(
+                            &modulename,
+                            &file_contents,
+                            errors,
+                        ))),
+                    ));
+                }
+
+                let macro_result = match result {
+                    Ok(m) => m,
+                    Err((None, err)) => return Err((None, err.into())),
+                    Err((Some(m), err)) => {
+                        macros.errors.push(pos::spanned(span, err.into()));
+                        m
+                    }
+                };
 
-                let earlier_errors_exist = errors_before != macros.errors.len();
                 self.importer.import(
                     compiler,
                     vm,
-                    earlier_errors_exist,
+                    has_errors,
                     &modulename,
                     &file_contents,
                     macro_result.expr,

src/lib.rs

@@ -46,6 +46,7 @@ pub use futures::Future;
 
 use either::Either;
 
+use std::error::Error as StdError;
 use std::result::Result as StdResult;
 use std::env;
 
@@ -55,7 +56,7 @@ use base::error::{Errors, InFile};
 use base::metadata::Metadata;
 use base::symbol::{Symbol, SymbolModule, Symbols};
 use base::types::{ArcType, TypeCache};
-use base::pos::{self, Span};
+use base::pos::{BytePos, Span, Spanned};
 
 use vm::Variants;
 use vm::api::{Getable, Hole, OpaqueValue, VmType};
@@ -95,8 +96,13 @@ quick_error! {
             from()
         }
         /// Error found when expanding macros
-        Macro(err: macros::SpannedError) {
-            description(err.value.description())
+        Macro(err: InFile<macros::Error>) {
+            description(err.description())
+            display("{}", err)
+            from()
+        }
+        Other(err: Box<StdError + Send + Sync>) {
+            description(err.description())
             display("{}", err)
             from()
         }
@@ -114,25 +120,21 @@ impl From<String> for Error {
     }
 }
 
-impl From<Errors<macros::SpannedError>> for Error {
-    fn from(mut errors: Errors<macros::SpannedError>) -> Error {
+impl From<Errors<Spanned<macros::Error, BytePos>>> for Error {
+    fn from(mut errors: Errors<Spanned<macros::Error, BytePos>>) -> Error {
         if errors.len() == 1 {
             let err = errors.pop().unwrap();
-            let span = err.span;
             match err.value.downcast::<Error>() {
                 Ok(err) => *err,
-                Err(err) => Error::Macro(pos::spanned(span, err)),
+                Err(err) => Error::Other(err),
             }
         } else {
             Error::Multiple(
                 errors
                     .into_iter()
-                    .map(|err| {
-                        let span = err.span;
-                        match err.value.downcast::<Error>() {
-                            Ok(err) => *err,
-                            Err(err) => Error::Macro(pos::spanned(span, err)),
-                        }
+                    .map(|err| match err.value.downcast::<Error>() {
+                        Ok(err) => *err,
+                        Err(err) => Error::Other(err),
                     })
                     .collect(),
             )
@@ -385,7 +387,7 @@ impl Compiler {
         FutureValue::from(
             import
                 .load_module(self, vm, &mut macros, &module_name, Span::default())
-                .map_err(|(_, err)| pos::spanned(Span::default(), err).into())
+                .map_err(|(_, err)| Error::Other(err))
                 .and_then(|_| Ok(macros.finish()?)),
         ).boxed()
     }

tests/ui.rs

@@ -0,0 +1,16 @@
+extern crate gluon;
+
+use gluon::{new_vm, Compiler};
+
+#[test]
+fn macro_error_with_line_column_info() {
+    let thread = new_vm();
+    let result = Compiler::new().run_expr::<()>(&thread, "test", "import! undefined");
+    assert_eq!(
+        result.unwrap_err().to_string(),
+        r#"test:Line: 1, Column: 9: Could not find module 'undefined'
+import! undefined
+        ^~~~~~~~~
+"#
+    );
+}