fix: Provide the type of imported modules with errors

Marwes · Dec 27, 2020 · d3bfc59 · d3bfc59
1 parent f39b396
commit d3bfc59
Show file tree

Hide file tree

Showing 10 changed files with 221 additions and 91 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -75,10 +75,11 @@ little-skeptic = { version = "0.15.0", optional = true }
 walkdir = "2"
 
 [dev-dependencies]
+anyhow = "1"
 criterion = "0.3"
 collect-mac = "0.1.0"
 env_logger = "0.7"
-anyhow = "1"
+expect-test = "1"
 thiserror = "1"
 insta = "0.16"
 pretty_assertions = "0.6"

diff --git a/base/src/error.rs b/base/src/error.rs
@@ -374,3 +374,63 @@ impl AsDiagnostic for Box<dyn ::std::error::Error + Send + Sync> {
         Diagnostic::error().with_message(self.to_string())
     }
 }
+
+pub type SalvageResult<T, E> = std::result::Result<T, Salvage<T, E>>;
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct Salvage<T, E> {
+    pub value: Option<T>,
+    pub error: E,
+}
+
+impl<T, E> fmt::Display for Salvage<T, E>
+where
+    E: fmt::Display,
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.error)
+    }
+}
+
+impl<T, E> Salvage<T, E> {
+    pub fn map<U>(self, f: impl FnOnce(T) -> U) -> Salvage<U, E> {
+        Salvage {
+            value: self.value.map(f),
+            error: self.error,
+        }
+    }
+
+    pub fn map_err<U>(self, f: impl FnOnce(E) -> U) -> Salvage<T, U> {
+        Salvage {
+            value: self.value,
+            error: f(self.error),
+        }
+    }
+
+    pub fn get_value(self) -> std::result::Result<T, E> {
+        self.value.ok_or(self.error)
+    }
+
+    pub fn err_into<F>(self) -> Salvage<T, F>
+    where
+        F: From<E>,
+    {
+        let Salvage { value, error } = self;
+        Salvage {
+            value,
+            error: error.into(),
+        }
+    }
+}
+
+impl<T, E> From<E> for Salvage<T, E> {
+    fn from(error: E) -> Self {
+        Salvage { value: None, error }
+    }
+}
+
+impl<T, E> From<Salvage<T, InFile<E>>> for InFile<E> {
+    fn from(s: Salvage<T, InFile<E>>) -> Self {
+        s.error
+    }
+}
diff --git a/check/tests/support/mod.rs b/check/tests/support/mod.rs
@@ -439,13 +439,18 @@ macro_rules! test_check {
 }
 
 macro_rules! test_check_err {
-    ($name:ident, $source:expr, $($id: pat),+) => {
+    ($name:ident, $source:expr, $($id: pat),+ $(,)? $(=> $ty: expr)?) => {
         #[test]
         fn $name() {
             let _ = env_logger::try_init();
             let text = $source;
-            let result = support::typecheck(text);
+            let (expr, result) = support::typecheck_expr(text);
             assert_err!([$source] result, $($id),+);
+            $(
+                use crate::base::ast::Typed;
+                assert_eq!(expr.expr().env_type_of(&crate::base::ast::EmptyEnv::default()).to_string(), $ty);
+            )?
+            let _ = expr;
         }
     };
 }

diff --git a/src/compiler_pipeline.rs b/src/compiler_pipeline.rs
@@ -9,7 +9,6 @@
 
 use std::{
     borrow::{Borrow, BorrowMut, Cow},
-    fmt,
     result::Result as StdResult,
     sync::Arc,
 };
@@ -44,58 +43,9 @@ use crate::{
 pub type BoxFuture<'vm, T, E> =
     std::pin::Pin<Box<dyn futures::Future<Output = StdResult<T, E>> + Send + 'vm>>;
 
-pub type SalvageResult<T, E = Error> = StdResult<T, Salvage<T, E>>;
+pub type SalvageResult<T, E = Error> = crate::base::error::SalvageResult<T, E>;
 
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub struct Salvage<T, E> {
-    pub value: Option<T>,
-    pub error: E,
-}
-
-impl<T, E> fmt::Display for Salvage<T, E>
-where
-    E: fmt::Display,
-{
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "{}", self.error)
-    }
-}
-
-impl<T, E> Salvage<T, E> {
-    pub fn map<U>(self, f: impl FnOnce(T) -> U) -> Salvage<U, E> {
-        Salvage {
-            value: self.value.map(f),
-            error: self.error,
-        }
-    }
-
-    pub fn get_value(self) -> std::result::Result<T, E> {
-        self.value.ok_or(self.error)
-    }
-
-    pub fn err_into<F>(self) -> Salvage<T, F>
-    where
-        F: From<E>,
-    {
-        let Salvage { value, error } = self;
-        Salvage {
-            value,
-            error: error.into(),
-        }
-    }
-}
-
-impl<T, E> From<E> for Salvage<T, E> {
-    fn from(error: E) -> Self {
-        Salvage { value: None, error }
-    }
-}
-
-impl<T, E> From<Salvage<T, InFile<E>>> for InFile<E> {
-    fn from(s: Salvage<T, InFile<E>>) -> Self {
-        s.error
-    }
-}
+pub use crate::base::error::Salvage;
 
 impl<T> From<Salvage<T, Error>> for Error {
     fn from(s: Salvage<T, Error>) -> Self {
@@ -648,19 +598,20 @@ where
         ) {
             Ok(typ) => typ,
             Err(error) => {
+                let typ = expr
+                    .borrow_mut()
+                    .expr()
+                    .try_type_of(&env(&*compiler.database))
+                    .unwrap_or_else(|_| thread.global_env().type_cache().error());
                 return Err(Salvage {
                     value: Some(TypecheckValue {
-                        typ: expr
-                            .borrow_mut()
-                            .expr()
-                            .try_type_of(&env(&*compiler.database))
-                            .unwrap_or_else(|_| thread.global_env().type_cache().error()),
+                        typ,
                         expr,
                         metadata_map,
                         metadata,
                     }),
                     error,
-                })
+                });
             }
         };
 

diff --git a/src/import.rs b/src/import.rs
@@ -32,14 +32,14 @@ use crate::base::{
 use crate::vm::{
     self,
     gc::Trace,
-    macros::{Error as MacroError, Macro, MacroExpander, MacroFuture},
+    macros::{Error as MacroError, LazyMacroResult, Macro, MacroExpander, MacroFuture},
     thread::{RootedThread, Thread},
     vm::VmEnv,
     ExternLoader, ExternModule,
 };
 
 use crate::{
-    compiler_pipeline::SalvageResult,
+    compiler_pipeline::{Salvage, SalvageResult},
     query::{AsyncCompilation, Compilation, CompilerDatabase},
     IoError, ModuleCompiler, ThreadExt,
 };
@@ -99,7 +99,17 @@ impl Importer for DefaultImporter {
         _vm: &Thread,
         modulename: &str,
     ) -> SalvageResult<ArcType> {
-        let value = compiler.database.global(modulename.to_string()).await?;
+        let result = compiler.database.global(modulename.to_string()).await;
+        // Forcibly load module_type so we can salvage a type for the error if necessary
+        let _ = compiler
+            .database
+            .module_type(modulename.to_string(), None)
+            .await;
+
+        let value = result.map_err(|error| {
+            let value = compiler.database.peek_module_type(modulename);
+            Salvage { value, error }
+        })?;
         Ok(value.typ)
     }
 }
@@ -534,38 +544,54 @@ where
                     let result = std::panic::AssertUnwindSafe(db.import(modulename))
                         .catch_unwind()
                         .await
-                        .map(|r| r.map_err(|err| MacroError::message(err.to_string())))
+                        .map(|r| {
+                            r.map_err(|salvage| {
+                                salvage.map_err(|err| MacroError::message(err.to_string()))
+                            })
+                        })
                         .unwrap_or_else(|err| {
-                            Err(MacroError::message(
+                            Err(Salvage::from(MacroError::message(
                                 err.downcast::<String>()
                                     .map(|s| *s)
                                     .or_else(|e| e.downcast::<&str>().map(|s| String::from(&s[..])))
                                     .unwrap_or_else(|_| "Unknown panic".to_string()),
-                            ))
+                            )))
                         });
                     // Drop the database before sending the result, otherwise the forker may drop before the forked database
                     drop(db);
                     let _ = tx.send(result);
                 }))
                 .unwrap();
             return Box::pin(async move {
-                Ok(From::from(move || {
-                    rx.map(move |r| {
-                        r.unwrap_or_else(|err| Err(MacroError::new(Error::String(err.to_string()))))
-                            .map(move |id| pos::spanned(span, Expr::Ident(id)).into())
-                    })
+                Ok(LazyMacroResult::from(move || {
+                    async move {
+                        rx.await
+                            .unwrap_or_else(|err| {
+                                Err(Salvage::from(MacroError::new(Error::String(
+                                    err.to_string(),
+                                ))))
+                            })
+                            .map(|id| pos::spanned(span, Expr::Ident(id)))
+                            .map_err(|salvage| {
+                                salvage.map(|id| pos::spanned(span, Expr::Ident(id)))
+                            })
+                    }
                     .boxed()
                 }))
             });
         }
 
         Box::pin(async move {
-            Ok(From::from(move || {
+            Ok(LazyMacroResult::from(move || {
                 async move {
                     let result = db
                         .import(modulename)
                         .await
-                        .map_err(|err| MacroError::message(err.to_string()))
+                        .map_err(|salvage| {
+                            salvage
+                                .map(|id| pos::spanned(span, Expr::Ident(id)))
+                                .map_err(|err| MacroError::message(err.to_string()))
+                        })
                         .map(move |id| pos::spanned(span, Expr::Ident(id)));
                     drop(db);
                     result

diff --git a/src/lib.rs b/src/lib.rs
@@ -651,7 +651,10 @@ pub trait ThreadExt: Send + Sync {
         }
         let mut db = vm.get_database();
 
-        db.import(module_name).await.map(|_| ())
+        db.import(module_name)
+            .await
+            .map(|_| ())
+            .map_err(|err| err.error)
     }
 
     /// Loads `filename` and compiles and runs its input by calling `load_script`