refactor: address package loader review feedback

README.md

@@ -63,6 +63,8 @@ the native bootstrap accepts either a no-argument `main` or `main proc(init proc
 
 source loading uses the supplied `io.Io` for opening, reading, and closing files. `io.open` accepts a borrowed zero-terminated slice (`[;0]u8`), including `init.args.values[index]` without copying its storage. the bootstrap supports checked sentinel-slice construction with `pointer[..length;0]`. `io.read_all(allocator, reader, maximum)` reads to EOF into a caller-owned `ArrayList(u8)`; release it with `arraylist.deinit`. it frees partial storage on error and consumes one extra byte to distinguish an exact-size input from a limit violation. the driver maps `limit_exceeded` to `source_too_large` at its `u32` source-size limit.
 
+`@std/fs` contains the Darwin filesystem operations used by source loading: `canonical` returns an owned symlink-resolved path (free with the same allocator), while `kind` and `exists` classify paths without exposing C mode bits. `open_directory` returns an owned handle released with `close_directory`; `next` returns a borrowed entry whose name expires on the next call or close. enumeration order is unspecified. `read_file(allocator, runtime, path, maximum)` reads a regular file through the supplied `io.Io`, closes it on success/error, and returns caller-owned bytes released with `arraylist.deinit`; exceeding the bound returns `limit_exceeded`. metadata and directory discovery use the host filesystem. the compiler's `@source/loader` retains project-root/import rules, production-file filtering, deterministic sorting and the `u32` source-size limit, with no direct libc calls.
+
 a recursive example is in [programs/factorial.hon](programs/factorial.hon); `./build/honey run programs/factorial.hon` prints `120`.
 
 an iterative example is in [programs/iterative_factorial.hon](programs/iterative_factorial.hon); `./build/honey run programs/iterative_factorial.hon` also prints `120`.

source/ast/ast.hon

@@ -155,6 +155,7 @@ Tree :: struct {
     root NodeId
 }
 
+#! transient projection, not stored per import; test_only decodes an existing Node payload word.
 ImportView :: struct {
     span Span
     path Span

source/compilation/compilation.hon

@@ -3,6 +3,7 @@ import "@std/debug"
 import "@std/arraylist"
 import "@std/hashmap"
 import "@std/io"
+import "@std/fs"
 import "@source/strpool"
 import "@source/diagnostics"
 import "@source/lexer"
@@ -165,7 +166,7 @@ load proc(
     path []u8,
     diagnostic_store @mut diagnostics.Store,
 ) PackageId ! Error {
-    canonical :: try loader.canonical(compilation.allocator, path)
+    canonical :: try fs.canonical(compilation.allocator, path)
     defer mem.free(compilation.allocator, canonical)
     directory :: try loader.directory(compilation.allocator, canonical)
     transferred := false
@@ -173,7 +174,7 @@ load proc(
     project_root :: try loader.project_root(compilation.allocator, directory)
     defer mem.free(compilation.allocator, project_root)
     entry_file ?[]u8 := null
-    if (!try loader.is_directory(compilation.allocator, canonical)) entry_file = canonical
+    if (try fs.kind(compilation.allocator, canonical) != .directory) entry_file = canonical
     transferred = true
     return try load_package(compilation, runtime, project_root, directory, entry_file, null, diagnostic_store)
 }
@@ -261,7 +262,7 @@ load_package proc(
                 }
                 owned := true
                 defer { if (owned) mem.free(compilation.allocator, resolved) }
-                if (try loader.is_directory(compilation.allocator, resolved)) {
+                if (try fs.kind(compilation.allocator, resolved) == .directory) {
                     owned = false
                     target = try load_package(compilation, runtime, project_root, resolved, null, imported.path, diagnostic_store)
                 } else {

source/loader/loader.hon

@@ -1,20 +1,15 @@
-import "@ffi/c"
 import "@std/mem"
 import "@std/io"
+import "@std/fs"
 import "@std/arraylist"
 
-PathError :: enum { invalid_path, path_failed, directory_failed, source_too_large }
+SourceError :: enum { source_too_large }
-Error :: alias mem.AllocError | PathError | io.OpenError | io.CloseError | io.ReadError
 
-@hide:file
+Error :: alias fs.Error
-terminated proc(allocator mem.Allocator, text []u8) []mut u8 ! (mem.AllocError | PathError) {
+    | SourceError
-    if (text.len == 0 or text.len == maxval!(usize)) return .invalid_path
+    | io.OpenError
-    for (text) |byte| if (byte == 0) return .invalid_path
+    | io.CloseError
-    bytes :: try mem.alloc(u8, allocator, text.len + 1)
+    | io.ReadError
-    memcopy!(bytes[..text.len], text)
-    bytes[text.len] = 0
-    return bytes
-}
 
 @hide:file
 copy proc(allocator mem.Allocator, text []u8) []mut u8 ! mem.AllocError {
@@ -23,53 +18,17 @@ copy proc(allocator mem.Allocator, text []u8) []mut u8 ! mem.AllocError {
     return bytes
 }
 
-#! caller owns the canonical absolute path; release it with mem.free using `allocator`.
-canonical proc(allocator mem.Allocator, path []u8) []mut u8 ! Error {
-    input :: try terminated(allocator, path)
-    defer mem.free(allocator, input)
-    # Darwin realpath requires a PATH_MAX-sized caller buffer (including its terminator).
-    buffer [1024]mut u8 := undefined
-    if (c.realpath(ptrcast!(input.ptr), ptrcast!(buffer[..].ptr)) == null) {
-        if (c.__error()?^ == c.ENOMEM) return .out_of_memory
-        return .path_failed
-    }
-    length := usize(0)
-    while (length < buffer.len and buffer[length] != 0) length += 1
-    if (length == buffer.len) return .path_failed
-    return try copy(allocator, buffer[..length])
-}
-
-@hide:file
-mode proc(allocator mem.Allocator, path []u8) c_ushort ! Error {
-    input :: try terminated(allocator, path)
-    defer mem.free(allocator, input)
-    result [1]mut c.Stat := undefined
-    if (c.stat(ptrcast!(input.ptr), result[..].ptr) == 0)
-        return result[0].st_mode
-
-    error :: c.__error()?^
-    if (error == c.ENOMEM) return .out_of_memory
-    if (error == c.ENOENT or error == c.ENOTDIR) return 0
-
-    return .path_failed
-}
-
-is_directory proc(allocator mem.Allocator, path []u8) bool ! Error {
-    return ((try mode(allocator, path)) & c_ushort(61440)) == 16384
-}
-
 #! returns an owned canonical package directory for either a file or directory input.
 directory proc(allocator mem.Allocator, path []u8) []mut u8 ! Error {
-    resolved :: try canonical(allocator, path)
+    resolved :: try fs.canonical(allocator, path)
     errdefer mem.free(allocator, resolved)
-    if (try is_directory(allocator, resolved)) return resolved
+    if (try fs.kind(allocator, resolved) == .directory) return resolved
 
     end := resolved.len
     while (end > 1 and resolved[end - 1] != '/') end -= 1
     parent_text :: if (end == 1) resolved[..1] else resolved[..end - 1]
     parent :: try copy(allocator, parent_text)
     mem.free(allocator, resolved)
-
     return parent
 }
 
@@ -81,10 +40,8 @@ join proc(allocator mem.Allocator, base, name []u8) []mut u8 ! mem.AllocError {
 
     bytes :: try mem.alloc(u8, allocator, base.len + separator + name.len)
     memcopy!(bytes[..base.len], base)
-
     if (separator != 0) bytes[base.len] = '/'
     memcopy!(bytes[base.len + separator..], name)
-
     return bytes
 }
 
@@ -92,14 +49,7 @@ join proc(allocator mem.Allocator, base, name []u8) []mut u8 ! mem.AllocError {
 exists proc(allocator mem.Allocator, base, name []u8) bool ! Error {
     path :: try join(allocator, base, name)
     defer mem.free(allocator, path)
-    input :: try terminated(allocator, path)
+    return try fs.exists(allocator, path)
-    defer mem.free(allocator, input)
-    if (c.access(ptrcast!(input.ptr), 0) == 0) return true
-
-    error :: c.__error()?^
-    if (error == c.ENOENT or error == c.ENOTDIR) return false
-
-    return .path_failed
 }
 
 #! finds the nearest ancestor build.hon/build.bro, otherwise uses the supplied directory.
@@ -111,7 +61,6 @@ project_root proc(allocator mem.Allocator, package_dir []u8) []mut u8 ! Error {
             return try copy(allocator, base)
 
         if (end == 1) break
-
         while (end > 1 and package_dir[end - 1] != '/') end -= 1
         if (end > 1) end -= 1
     }
@@ -131,7 +80,7 @@ resolve proc(allocator mem.Allocator, root_dir, package_dir, import_path []u8) [
     }
     joined :: try join(allocator, base, path)
     defer mem.free(allocator, joined)
-    return try canonical(allocator, joined)
+    return try fs.canonical(allocator, joined)
 }
 
 @hide:file
@@ -150,47 +99,33 @@ before proc(left, right []u8) bool {
 
 #! lists production sources in bytewise order; caller frees every path and the list.
 files proc(allocator mem.Allocator, package_dir []u8) arraylist.ArrayList([]u8) ! Error {
-    input :: try terminated(allocator, package_dir)
+    handle := try fs.open_directory(allocator, package_dir)
-    defer mem.free(allocator, input)
-    opened :: c.opendir(ptrcast!(input.ptr))
-    if (opened == null) {
-        if (c.__error()?^ == c.ENOMEM) return .out_of_memory
-        return .directory_failed
-    }
-    handle :: opened?
     closing := false
-    defer { if (!closing) _ = c.closedir(handle) }
+    defer { if (!closing) fs.close_directory(&handle) catch |_| {} }
     result := arraylist.init([]u8, allocator)
     errdefer {
         for (result.items) |path| mem.free(allocator, path)
         arraylist.deinit(&result)
     }
-    while true {
+    while (try fs.next(&handle)) |entry| {
-        c.__error()?^ = 0
+        if (
-        next :: c.readdir(handle)
+            entry.kind != .file
-        if (next == null) {
+            and entry.kind != .link
-            if (c.__error()?^ != 0) return .directory_failed
+            and entry.kind != .unknown
-            break
+        ) continue
-        }
+        name :: entry.name
-        entry :: next?
-        # DT_REG, DT_LNK and DT_UNKNOWN; never open directories, sockets or FIFOs as sources.
-        if (entry.d_type != 8 and entry.d_type != 10 and entry.d_type != 0) continue
-        length :: usize(entry.d_namlen)
-        if (length >= 1024) return .directory_failed
-        pointer *u8 :: ptrcast!(entry.d_name[..].ptr)
-        name :: pointer[..length]
         if ((!ends_with(name, ".hon") and !ends_with(name, ".bro"))
             or ends_with(name, ".test.hon") or ends_with(name, ".test.bro")) continue
         path :: try join(allocator, package_dir, name)
         errdefer mem.free(allocator, path)
-        if (entry.d_type != 8 and ((try mode(allocator, path)) & c_ushort(61440)) != 32768) {
+        if (entry.kind != .file and try fs.kind(allocator, path) != .file) {
             mem.free(allocator, path)
             continue
         }
         try arraylist.append(&result, path)
     }
     closing = true
-    if (c.closedir(handle) != 0) return .directory_failed
+    try fs.close_directory(&handle)
     # in-place heapsort: deterministic O(n log n), no additional allocation.
     count := result.items.len
     start :: divtrunc!(count, 2)
@@ -219,23 +154,12 @@ sift proc(items []mut []u8, start, count usize) void {
     }
 }
 
-#! reads through the supplied IO provider; returned bytes belong to the caller.
+#! the compiler limits source size to its u32 span address space; storage remains caller-owned.
 read_source proc(allocator mem.Allocator, runtime io.Io, path []u8) arraylist.ArrayList(u8) ! Error {
-    # source input must be a regular file, not a FIFO/device that can block before reading.
+    return fs.read_file(allocator, runtime, path, usize(maxval!(u32))) catch |err| {
-    if (((try mode(allocator, path)) & c_ushort(61440)) != 32768) return .open_failed
-    input :: try terminated(allocator, path)
-    defer mem.free(allocator, input)
-    file :: try io.open(runtime, input.ptr[..path.len;0], .read_only)
-    closing := false
-    defer { if (!closing) io.close(file) catch |_| {} }
-    bytes := io.read_all(allocator, io.reader(file), usize(maxval!(u32))) catch |err| {
         match err {
             .limit_exceeded: return .source_too_large
             else: return err
         }
     }
-    errdefer arraylist.deinit(&bytes)
-    closing = true
-    try io.close(file)
-    return bytes
 }

source/main.hon

@@ -19,6 +19,7 @@ test import "@std/strmap"
 test import "@std/meta"
 test import "@std/io"
 test import "@std/process"
+test import "@std/fs"
 test import "@std/strconv"
 test import "@source/diagnostics"
 test import "@source/strpool"

std/fs/fs.hon

@@ -0,0 +1,142 @@
+import "@ffi/c"
+import "@std/mem"
+import "@std/io"
+import "@std/arraylist"
+
+PathError :: enum { invalid_path, path_failed, directory_failed }
+
+Error :: alias mem.AllocError | PathError
+ReadFileError :: alias Error | io.OpenError | io.CloseError | io.ReadError | io.ReadLimitError
+
+Kind :: enum { missing, file, directory, link, unknown, other }
+
+#! owns a native directory handle; release it exactly once with close_directory.
+Directory :: struct { handle ?*mut c.DIR }
+
+#! name borrows directory-owned storage until the next call to next or close_directory.
+#! enumeration may report link or unknown; kind follows links when classification is needed.
+Entry :: struct {
+    name []u8
+    kind Kind
+}
+
+@hide:file
+terminated proc(allocator mem.Allocator, text []u8) []mut u8 ! Error {
+    if (text.len == 0 or text.len == maxval!(usize)) return .invalid_path
+    for (text) |byte| if (byte == 0) return .invalid_path
+    bytes :: try mem.alloc(u8, allocator, text.len + 1)
+    memcopy!(bytes[..text.len], text)
+    bytes[text.len] = 0
+    return bytes
+}
+
+#! returns an owned absolute path with symlinks resolved; free it with the same allocator.
+canonical proc(allocator mem.Allocator, path []u8) []mut u8 ! Error {
+    input :: try terminated(allocator, path)
+    defer mem.free(allocator, input)
+    # Darwin realpath requires a PATH_MAX-sized caller buffer, including its terminator.
+    buffer [1024]mut u8 := undefined
+    if (c.realpath(ptrcast!(input.ptr), ptrcast!(buffer[..].ptr)) == null) {
+        if (c.__error()?^ == c.ENOMEM) return .out_of_memory
+        return .path_failed
+    }
+    length := usize(0)
+    while (length < buffer.len and buffer[length] != 0) length += 1
+    if (length == buffer.len) return .path_failed
+    owned :: try mem.alloc(u8, allocator, length)
+    memcopy!(owned, buffer[..length])
+    return owned
+}
+
+#! follows symlinks; nonexistent paths (including dangling links) return missing.
+kind proc(allocator mem.Allocator, path []u8) Kind ! Error {
+    input :: try terminated(allocator, path)
+    defer mem.free(allocator, input)
+    result [1]mut c.Stat := undefined
+    if (c.stat(ptrcast!(input.ptr), result[..].ptr) == 0) {
+        mode :: result[0].st_mode & c_ushort(61440)
+        if (mode == 16384) return .directory
+        if (mode == 32768) return .file
+        return .other
+    }
+    error :: c.__error()?^
+    if (error == c.ENOMEM) return .out_of_memory
+    if (error == c.ENOENT or error == c.ENOTDIR) return .missing
+    return .path_failed
+}
+
+exists proc(allocator mem.Allocator, path []u8) bool ! Error {
+    return (try kind(allocator, path)) != .missing
+}
+
+#! opens an owned directory handle; path is borrowed only during this call.
+open_directory proc(allocator mem.Allocator, path []u8) Directory ! Error {
+    input :: try terminated(allocator, path)
+    defer mem.free(allocator, input)
+    handle :: c.opendir(ptrcast!(input.ptr))
+    if (handle == null) {
+        if (c.__error()?^ == c.ENOMEM) return .out_of_memory
+        return .directory_failed
+    }
+    return Directory{ handle = handle }
+}
+
+#! returns the next borrowed entry, omitting . and ..; null marks the end.
+#! order is filesystem-defined. no allocation is performed; each name expires on the next call.
+next proc(directory @Directory) ?Entry ! Error {
+    if (directory.handle == null) return .directory_failed
+    while true {
+        c.__error()?^ = 0
+        item :: c.readdir(directory.handle)
+        if (item == null) {
+            if (c.__error()?^ == c.ENOMEM) return .out_of_memory
+            if (c.__error()?^ != 0) return .directory_failed
+            end ?Entry :: null
+            return end
+        }
+        entry :: item?
+        length :: usize(entry.d_namlen)
+        if (length >= 1024) return .directory_failed
+        pointer *u8 :: ptrcast!(entry.d_name[..].ptr)
+        name :: pointer[..length]
+        if (mem.eql(name, ".") or mem.eql(name, "..")) continue
+        entry_kind Kind :: match entry.d_type {
+            0: .unknown
+            4: .directory
+            8: .file
+            10: .link
+            else: .other
+        }
+        return Entry{ name = name, kind = entry_kind }
+    }
+}
+
+#! invalidates the handle, including when native close reports failure.
+close_directory proc(directory @mut Directory) void ! Error {
+    handle :: directory.handle
+    if (handle == null) return .directory_failed
+    directory.handle = null
+    if (c.closedir(handle) != 0) return .directory_failed
+}
+
+#! reads a regular file through runtime IO, accepting at most maximum bytes.
+#! follows symlinks and rejects devices/FIFOs before opening; metadata uses the host filesystem.
+#! returned storage belongs to the caller; release with arraylist.deinit. all errors close the file.
+read_file proc(
+    allocator mem.Allocator,
+    runtime io.Io,
+    path []u8,
+    maximum usize,
+) arraylist.ArrayList(u8) ! ReadFileError {
+    if (try kind(allocator, path) != .file) return .open_failed
+    input :: try terminated(allocator, path)
+    defer mem.free(allocator, input)
+    file :: try io.open(runtime, input.ptr[..path.len;0], .read_only)
+    closing := false
+    defer { if (!closing) io.close(file) catch |_| {} }
+    bytes := try io.read_all(allocator, io.reader(file), maximum)
+    errdefer arraylist.deinit(&bytes)
+    closing = true
+    try io.close(file)
+    return bytes
+}

std/fs/fs.test.hon

@@ -0,0 +1,28 @@
+import "@std/mem"
+import "@std/testing"
+
+rejects_empty_paths_and_embedded_nul_instead_of_truncating test {
+    paths [2][]u8 :: ["", ".\0not-part-of-the-path"]
+    for (paths) |path| {
+        resolved :: canonical(mem.c_allocator, path) catch |err| {
+            try testing.expect(err == .invalid_path)
+            continue
+        }
+        mem.free(mem.c_allocator, resolved)
+        return .expectation_failed
+    }
+}
+
+closed_directory_cannot_be_read_or_closed_again test {
+    directory := try open_directory(mem.c_allocator, ".")
+    try close_directory(&directory)
+    _ = next(&directory) catch |err| {
+        try testing.expect(err == .directory_failed)
+        close_directory(&directory) catch |close_error| {
+            try testing.expect(close_error == .directory_failed)
+            return
+        }
+        return .expectation_failed
+    }
+    return .expectation_failed
+}