FileId instead of Path + FileHash

8 files changed, 126 insertions, 163 deletions

diff --git a/src/eval/mod.rs b/src/eval/mod.rs
index ed8a81c8..cbb61153 100644
--- a/src/eval/mod.rs
+++ b/src/eval/mod.rs
@@ -22,28 +22,27 @@ pub use value::*;
 
 use std::collections::HashMap;
 use std::mem;
-use std::path::{Path, PathBuf};
+use std::path::Path;
 use std::rc::Rc;
 
 use crate::cache::Cache;
 use crate::diag::{Diag, DiagSet, Pass};
 use crate::eco::EcoString;
 use crate::geom::{Angle, Fractional, Length, Relative};
-use crate::loading::{FileHash, Loader};
+use crate::loading::{FileId, Loader};
 use crate::parse::parse;
 use crate::syntax::visit::Visit;
 use crate::syntax::*;
-use crate::util::PathExt;
 
 /// Evaluate a parsed source file into a module.
 pub fn eval(
     loader: &mut dyn Loader,
     cache: &mut Cache,
-    path: Option<&Path>,
+    location: FileId,
     ast: Rc<SyntaxTree>,
     scope: &Scope,
 ) -> Pass<Module> {
-    let mut ctx = EvalContext::new(loader, cache, path, scope);
+    let mut ctx = EvalContext::new(loader, cache, location, scope);
     let template = ast.eval(&mut ctx);
     let module = Module { scope: ctx.scopes.top, template };
     Pass::new(module, ctx.diags)
@@ -68,12 +67,10 @@ pub struct EvalContext<'a> {
     pub scopes: Scopes<'a>,
     /// Evaluation diagnostics.
     pub diags: DiagSet,
-    /// The location of the currently evaluated file.
-    pub path: Option<PathBuf>,
     /// The stack of imported files that led to evaluation of the current file.
-    pub route: Vec<FileHash>,
+    pub route: Vec<FileId>,
     /// A map of loaded module.
-    pub modules: HashMap<FileHash, Module>,
+    pub modules: HashMap<FileId, Module>,
 }
 
 impl<'a> EvalContext<'a> {
@@ -81,25 +78,15 @@ impl<'a> EvalContext<'a> {
     pub fn new(
         loader: &'a mut dyn Loader,
         cache: &'a mut Cache,
-        path: Option<&Path>,
+        location: FileId,
         scope: &'a Scope,
     ) -> Self {
-        let path = path.map(PathExt::normalize);
-
-        let mut route = vec![];
-        if let Some(path) = &path {
-            if let Some(hash) = loader.resolve(path) {
-                route.push(hash);
-            }
-        }
-
         Self {
             loader,
             cache,
             scopes: Scopes::new(Some(scope)),
             diags: DiagSet::new(),
-            path,
-            route,
+            route: vec![location],
             modules: HashMap::new(),
         }
     }
@@ -107,37 +94,30 @@ impl<'a> EvalContext<'a> {
     /// Resolve a path relative to the current file.
     ///
     /// Generates an error if the file is not found.
-    pub fn resolve(&mut self, path: &str, span: Span) -> Option<(PathBuf, FileHash)> {
-        let path = match &self.path {
-            Some(current) => current.parent()?.join(path),
-            None => PathBuf::from(path),
-        };
-
-        match self.loader.resolve(&path) {
-            Some(hash) => Some((path.normalize(), hash)),
-            None => {
-                self.diag(error!(span, "file not found"));
-                None
-            }
-        }
+    pub fn resolve(&mut self, path: &str, span: Span) -> Option<FileId> {
+        let base = *self.route.last()?;
+        self.loader.resolve_from(base, Path::new(path)).or_else(|| {
+            self.diag(error!(span, "file not found"));
+            None
+        })
     }
 
     /// Process an import of a module relative to the current location.
-    pub fn import(&mut self, path: &str, span: Span) -> Option<FileHash> {
-        let (resolved, hash) = self.resolve(path, span)?;
+    pub fn import(&mut self, path: &str, span: Span) -> Option<FileId> {
+        let id = self.resolve(path, span)?;
 
         // Prevent cyclic importing.
-        if self.route.contains(&hash) {
+        if self.route.contains(&id) {
             self.diag(error!(span, "cyclic import"));
             return None;
         }
 
         // Check whether the module was already loaded.
-        if self.modules.get(&hash).is_some() {
-            return Some(hash);
+        if self.modules.get(&id).is_some() {
+            return Some(id);
         }
 
-        let buffer = self.loader.load_file(&resolved).or_else(|| {
+        let buffer = self.loader.load_file(id).or_else(|| {
             self.diag(error!(span, "failed to load file"));
             None
         })?;
@@ -154,8 +134,7 @@ impl<'a> EvalContext<'a> {
         let new_scopes = Scopes::new(self.scopes.base);
         let old_scopes = mem::replace(&mut self.scopes, new_scopes);
         let old_diags = mem::replace(&mut self.diags, parsed.diags);
-        let old_path = mem::replace(&mut self.path, Some(resolved));
-        self.route.push(hash);
+        self.route.push(id);
 
         // Evaluate the module.
         let ast = Rc::new(parsed.output);
@@ -164,7 +143,6 @@ impl<'a> EvalContext<'a> {
         // Restore the old context.
         let new_scopes = mem::replace(&mut self.scopes, old_scopes);
         let new_diags = mem::replace(&mut self.diags, old_diags);
-        self.path = old_path;
         self.route.pop();
 
         // Put all diagnostics from the module on the import.
@@ -175,9 +153,9 @@ impl<'a> EvalContext<'a> {
 
         // Save the evaluated module.
         let module = Module { scope: new_scopes.top, template };
-        self.modules.insert(hash, module);
+        self.modules.insert(id, module);
 
-        Some(hash)
+        Some(id)
     }
 
     /// Add a diagnostic.
diff --git a/src/font.rs b/src/font.rs
index d63c4c1b..273c2914 100644
--- a/src/font.rs
+++ b/src/font.rs
@@ -1,18 +1,19 @@
 //! Font handling.
 
-use std::collections::HashMap;
+use std::collections::{hash_map::Entry, HashMap};
 use std::fmt::{self, Debug, Display, Formatter};
 use std::ops::Add;
+use std::rc::Rc;
 
 use decorum::N64;
 use serde::{Deserialize, Serialize};
 
 use crate::geom::Length;
-use crate::loading::{Buffer, Loader};
+use crate::loading::{FileId, Loader};
 
 /// A font face.
 pub struct Face {
-    buffer: Buffer,
+    buffer: Rc<Vec<u8>>,
     index: u32,
     ttf: rustybuzz::Face<'static>,
     units_per_em: f64,
@@ -33,7 +34,7 @@ pub struct LineMetrics {
 
 impl Face {
     /// Parse a font face from a buffer and collection index.
-    pub fn new(buffer: Buffer, index: u32) -> Option<Self> {
+    pub fn new(buffer: Rc<Vec<u8>>, index: u32) -> Option<Self> {
         // SAFETY:
         // - The slices's location is stable in memory:
         //   - We don't move the underlying vector
@@ -88,7 +89,7 @@ impl Face {
     }
 
     /// The underlying buffer.
-    pub fn buffer(&self) -> &Buffer {
+    pub fn buffer(&self) -> &Rc<Vec<u8>> {
         &self.buffer
     }
 
@@ -204,6 +205,7 @@ impl Add for Em {
 pub struct FontCache {
     faces: Vec<Option<Face>>,
     families: HashMap<String, Vec<FaceId>>,
+    buffers: HashMap<FileId, Rc<Vec<u8>>>,
     on_load: Option<Box<dyn Fn(FaceId, &Face)>>,
 }
 
@@ -222,7 +224,12 @@ impl FontCache {
                 .or_insert_with(|| vec![id]);
         }
 
-        Self { faces, families, on_load: None }
+        Self {
+            faces,
+            families,
+            buffers: HashMap::new(),
+            on_load: None,
+        }
     }
 
     /// Query for and load the font face from the given `family` that most
@@ -270,12 +277,23 @@ impl FontCache {
         let idx = id.0 as usize;
         let slot = &mut self.faces[idx];
         if slot.is_none() {
-            let index = infos[idx].index;
-            let buffer = loader.load_face(idx)?;
-            let face = Face::new(buffer, index)?;
+            let FaceInfo { file, index, .. } = infos[idx];
+
+            // Check the buffer cache since multiple faces may
+            // refer to the same data (font collection).
+            let buffer = match self.buffers.entry(file) {
+                Entry::Occupied(entry) => entry.into_mut(),
+                Entry::Vacant(entry) => {
+                    let buffer = loader.load_file(file)?;
+                    entry.insert(Rc::new(buffer))
+                }
+            };
+
+            let face = Face::new(Rc::clone(buffer), index)?;
             if let Some(callback) = &self.on_load {
                 callback(id, &face);
             }
+
             *slot = Some(face);
         }
 
@@ -322,14 +340,16 @@ impl FaceId {
 /// Properties of a single font face.
 #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)]
 pub struct FaceInfo {
+    /// The font file.
+    pub file: FileId,
+    /// The collection index in the font file.
+    pub index: u32,
     /// The typographic font family this face is part of.
     pub family: String,
     /// Properties that distinguish this face from other faces in the same
     /// family.
     #[serde(flatten)]
     pub variant: FontVariant,
-    /// The collection index in the font file.
-    pub index: u32,
 }
 
 /// Properties that distinguish a face from other faces in the same family.
diff --git a/src/image.rs b/src/image.rs
index f4617c5a..28283259 100644
--- a/src/image.rs
+++ b/src/image.rs
@@ -3,13 +3,12 @@
 use std::collections::{hash_map::Entry, HashMap};
 use std::fmt::{self, Debug, Formatter};
 use std::io::Cursor;
-use std::path::Path;
 
 use image::io::Reader as ImageReader;
 use image::{DynamicImage, GenericImageView, ImageFormat};
 use serde::{Deserialize, Serialize};
 
-use crate::loading::Loader;
+use crate::loading::{FileId, Loader};
 
 /// A loaded image.
 pub struct Image {
@@ -69,11 +68,10 @@ impl ImageCache {
     }
 
     /// Load and decode an image file from a path.
-    pub fn load(&mut self, loader: &mut dyn Loader, path: &Path) -> Option<ImageId> {
-        let hash = loader.resolve(path)?;
-        let id = ImageId(hash.into_raw());
+    pub fn load(&mut self, loader: &mut dyn Loader, file: FileId) -> Option<ImageId> {
+        let id = ImageId(file.into_raw());
         if let Entry::Vacant(entry) = self.images.entry(id) {
-            let buffer = loader.load_file(path)?;
+            let buffer = loader.load_file(file)?;
             let image = Image::parse(&buffer)?;
             if let Some(callback) = &self.on_load {
                 callback(id, &image);
diff --git a/src/lib.rs b/src/lib.rs
index 000d61d3..d1f43380 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -49,7 +49,6 @@ pub mod pretty;
 pub mod syntax;
 pub mod util;
 
-use std::path::Path;
 use std::rc::Rc;
 
 use crate::cache::Cache;
@@ -57,7 +56,7 @@ use crate::diag::Pass;
 use crate::eval::Scope;
 use crate::exec::State;
 use crate::layout::Frame;
-use crate::loading::Loader;
+use crate::loading::{FileId, Loader};
 
 /// Process source code directly into a collection of layouted frames.
 ///
@@ -65,12 +64,11 @@ use crate::loading::Loader;
 /// - The `loader` is used to load fonts, images and other source files.
 /// - The `cache` stores things that are reusable across several compilations
 ///   like loaded fonts, decoded images and layouting artifacts.
-/// - The `path` should point to the source file if `src` comes from the file
-///   system and is used to resolve relative paths (for importing and image
-///   loading).
+/// - The `location` is the file id of the source file and is used to resolve
+///   relative paths (for importing and image loading).
 /// - The `src` is the _Typst_ source code to typeset.
-/// - The `scope` contains definitions that are available everywhere,
-///   typically the standard library.
+/// - The `scope` contains definitions that are available everywhere, typically
+///   the standard library.
 /// - The `state` defines initial properties for page size, font selection and
 ///   so on.
 ///
@@ -80,13 +78,13 @@ use crate::loading::Loader;
 pub fn typeset(
     loader: &mut dyn Loader,
     cache: &mut Cache,
-    path: Option<&Path>,
+    location: FileId,
     src: &str,
     scope: &Scope,
     state: State,
 ) -> Pass<Vec<Rc<Frame>>> {
     let ast = parse::parse(src);
-    let module = eval::eval(loader, cache, path, Rc::new(ast.output), scope);
+    let module = eval::eval(loader, cache, location, Rc::new(ast.output), scope);
     let tree = exec::exec(&module.output.template, state);
     let frames = layout::layout(loader, cache, &tree.output);
 
diff --git a/src/library/elements.rs b/src/library/elements.rs
index dd2da877..03fccfe7 100644
--- a/src/library/elements.rs
+++ b/src/library/elements.rs
@@ -15,8 +15,8 @@ pub fn image(ctx: &mut EvalContext, args: &mut FuncArgs) -> Value {
 
     let mut node = None;
     if let Some(path) = &path {
-        if let Some((resolved, _)) = ctx.resolve(&path.v, path.span) {
-            if let Some(id) = ctx.cache.image.load(ctx.loader, &resolved) {
+        if let Some(file) = ctx.resolve(&path.v, path.span) {
+            if let Some(id) = ctx.cache.image.load(ctx.loader, file) {
                 node = Some(ImageNode { id, width, height });
             } else {
                 ctx.diag(error!(path.span, "failed to load image"));
diff --git a/src/loading/fs.rs b/src/loading/fs.rs
index 7fa1c120..ea33016c 100644
--- a/src/loading/fs.rs
+++ b/src/loading/fs.rs
@@ -1,39 +1,43 @@
-use std::collections::{hash_map::Entry, HashMap};
-use std::fs::File;
+use std::collections::HashMap;
+use std::fs::{self, File};
 use std::io;
 use std::path::{Path, PathBuf};
-use std::rc::Rc;
 
 use memmap2::Mmap;
 use same_file::Handle;
-use serde::{Deserialize, Serialize};
 use ttf_parser::{name_id, Face};
 use walkdir::WalkDir;
 
-use super::{Buffer, FileHash, Loader};
+use super::{FileId, Loader};
 use crate::font::{FaceInfo, FontStretch, FontStyle, FontVariant, FontWeight};
+use crate::util::PathExt;
 
 /// Loads fonts and images from the local file system.
 ///
 /// _This is only available when the `fs` feature is enabled._
-#[derive(Default, Debug, Clone, Serialize, Deserialize)]
+#[derive(Default, Debug, Clone)]
 pub struct FsLoader {
     faces: Vec<FaceInfo>,
-    files: Vec<PathBuf>,
-    #[serde(skip)]
-    cache: FileCache,
+    paths: HashMap<FileId, PathBuf>,
 }
 
-/// Maps from resolved file hashes to loaded file buffers.
-type FileCache = HashMap<FileHash, Buffer>;
-
 impl FsLoader {
     /// Create a new loader without any fonts.
     pub fn new() -> Self {
-        Self {
-            faces: vec![],
-            files: vec![],
-            cache: HashMap::new(),
+        Self { faces: vec![], paths: HashMap::new() }
+    }
+
+    /// Resolve a file id for a path.
+    pub fn resolve_path(&mut self, path: &Path) -> io::Result<FileId> {
+        let file = File::open(path)?;
+        let meta = file.metadata()?;
+        if meta.is_file() {
+            let handle = Handle::from_file(file)?;
+            let id = FileId(fxhash::hash64(&handle));
+            self.paths.insert(id, path.normalize());
+            Ok(id)
+        } else {
+            Err(io::Error::new(io::ErrorKind::Other, "not a file"))
         }
     }
 
@@ -149,17 +153,11 @@ impl FsLoader {
             stretch: FontStretch::from_number(face.width().to_number()),
         };
 
-        // Merge with an existing entry for the same family name.
-        self.faces.push(FaceInfo { family, variant, index });
-        self.files.push(path.to_owned());
+        let file = self.resolve_path(path)?;
+        self.faces.push(FaceInfo { file, index, family, variant });
 
         Ok(())
     }
-
-    /// Paths to font files, parallel to [`faces()`](Self::faces).
-    pub fn files(&self) -> &[PathBuf] {
-        &self.files
-    }
 }
 
 impl Loader for FsLoader {
@@ -167,30 +165,14 @@ impl Loader for FsLoader {
         &self.faces
     }
 
-    fn load_face(&mut self, idx: usize) -> Option<Buffer> {
-        self.load_file(&self.files[idx].clone())
+    fn resolve_from(&mut self, base: FileId, path: &Path) -> Option<FileId> {
+        let dir = self.paths[&base].parent()?;
+        let full = dir.join(path);
+        self.resolve_path(&full).ok()
     }
 
-    fn load_file(&mut self, path: &Path) -> Option<Buffer> {
-        let hash = self.resolve(path)?;
-        Some(Rc::clone(match self.cache.entry(hash) {
-            Entry::Occupied(entry) => entry.into_mut(),
-            Entry::Vacant(entry) => {
-                let buffer = std::fs::read(path).ok()?;
-                entry.insert(Rc::new(buffer))
-            }
-        }))
-    }
-
-    fn resolve(&self, path: &Path) -> Option<FileHash> {
-        let file = File::open(path).ok()?;
-        let meta = file.metadata().ok()?;
-        if meta.is_file() {
-            let handle = Handle::from_file(file).ok()?;
-            Some(FileHash::from_raw(fxhash::hash64(&handle)))
-        } else {
-            None
-        }
+    fn load_file(&mut self, id: FileId) -> Option<Vec<u8>> {
+        fs::read(&self.paths[&id]).ok()
     }
 }
 
@@ -203,7 +185,10 @@ mod tests {
         let mut loader = FsLoader::new();
         loader.search_path("fonts");
 
-        assert_eq!(loader.files, &[
+        let mut paths: Vec<_> = loader.paths.values().collect();
+        paths.sort();
+
+        assert_eq!(paths, [
             Path::new("fonts/EBGaramond-Bold.ttf"),
             Path::new("fonts/EBGaramond-BoldItalic.ttf"),
             Path::new("fonts/EBGaramond-Italic.ttf"),
diff --git a/src/loading/mod.rs b/src/loading/mod.rs
index 0e171796..c2f7ca39 100644
--- a/src/loading/mod.rs
+++ b/src/loading/mod.rs
@@ -7,44 +7,39 @@ mod fs;
 pub use fs::*;
 
 use std::path::Path;
-use std::rc::Rc;
 
-use crate::font::FaceInfo;
+use serde::{Deserialize, Serialize};
 
-/// A shared byte buffer.
-pub type Buffer = Rc<Vec<u8>>;
+use crate::font::FaceInfo;
 
 /// Loads resources from a local or remote source.
 pub trait Loader {
     /// Descriptions of all font faces this loader serves.
     fn faces(&self) -> &[FaceInfo];
 
-    /// Load the font face with the given index in [`faces()`](Self::faces).
-    fn load_face(&mut self, idx: usize) -> Option<Buffer>;
-
-    /// Load a file from a path.
-    fn load_file(&mut self, path: &Path) -> Option<Buffer>;
-
-    /// Resolve a hash for the file the path points to.
+    /// Resolve a `path` relative to a `base` file.
     ///
-    /// This should return the same hash for all paths pointing to the same file
+    /// This should return the same id for all paths pointing to the same file
     /// and `None` if the file does not exist.
-    fn resolve(&self, path: &Path) -> Option<FileHash>;
+    fn resolve_from(&mut self, base: FileId, path: &Path) -> Option<FileId>;
+
+    /// Load a file by id.
+    fn load_file(&mut self, id: FileId) -> Option<Vec<u8>>;
 }
 
-/// A file hash that can be [resolved](Loader::resolve) from a path.
+/// A file id that can be [resolved](Loader::resolve_from) from a path.
 ///
 /// Should be the same for all paths pointing to the same file.
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
-pub struct FileHash(u64);
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+pub struct FileId(u64);
 
-impl FileHash {
-    /// Create an file hash from a raw hash value.
+impl FileId {
+    /// Create a file id from a raw value.
     pub fn from_raw(v: u64) -> Self {
         Self(v)
     }
 
-    /// Convert into the raw underlying hash value.
+    /// Convert into the raw underlying value.
     pub fn into_raw(self) -> u64 {
         self.0
     }
@@ -58,15 +53,11 @@ impl Loader for BlankLoader {
         &[]
     }
 
-    fn load_face(&mut self, _: usize) -> Option<Buffer> {
-        None
-    }
-
-    fn load_file(&mut self, _: &Path) -> Option<Buffer> {
+    fn resolve_from(&mut self, _: FileId, _: &Path) -> Option<FileId> {
         None
     }
 
-    fn resolve(&self, _: &Path) -> Option<FileHash> {
+    fn load_file(&mut self, _: FileId) -> Option<Vec<u8>> {
         None
     }
 }
diff --git a/src/main.rs b/src/main.rs
index d0762b3e..03680204 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -16,19 +16,16 @@ fn main() -> anyhow::Result<()> {
     loader.search_path("fonts");
     loader.search_system();
 
-    // Resolve the canonical path because the compiler needs it for module
-    // loading.
+    // Determine source and destination path.
     let src_path = Path::new(&args[1]);
+    let dest_path = if let Some(arg) = args.get(2) {
+        PathBuf::from(arg)
+    } else {
+        let name = src_path
+            .file_name()
+            .ok_or_else(|| anyhow!("source path is not a file"))?;
 
-    // Find out the file name to create the output file.
-    let name = src_path
-        .file_name()
-        .ok_or_else(|| anyhow!("source path is not a file"))?;
-
-    let dest_path = if args.len() <= 2 {
         Path::new(name).with_extension("pdf")
-    } else {
-        PathBuf::from(&args[2])
     };
 
     // Ensure that the source file is not overwritten.
@@ -36,6 +33,9 @@ fn main() -> anyhow::Result<()> {
         bail!("source and destination files are the same");
     }
 
+    // Resolve the file id of the source file.
+    let src_id = loader.resolve_path(src_path).context("source file not found")?;
+
     // Read the source.
     let src = fs::read_to_string(&src_path)
         .map_err(|_| anyhow!("failed to read source file"))?;
@@ -44,14 +44,7 @@ fn main() -> anyhow::Result<()> {
     let mut cache = typst::cache::Cache::new(&loader);
     let scope = typst::library::new();
     let state = typst::exec::State::default();
-    let pass = typst::typeset(
-        &mut loader,
-        &mut cache,
-        Some(&src_path),
-        &src,
-        &scope,
-        state,
-    );
+    let pass = typst::typeset(&mut loader, &mut cache, src_id, &src, &scope, state);
 
     // Print diagnostics.
     let map = typst::parse::LineMap::new(&src);