Skip to content

Commit

Permalink
Refactor to raise error values instead of yielding them.
Browse files Browse the repository at this point in the history
This code previously contained a weird/brittle pattern of
functions that use `yield` before every possible `error!`,
in order to convey an error code with each error,
as a workaround for the fact that Savi didn't yet
support error values.

Now that [the latest version of Savi]
_does_ support error values, we can use them here.

Note that there is a separate issue uncovered here
for the Savi compiler showing "<unsatisfiable>" errors
when every branch of a choice-inside-a-choice block jumps away.
For now this code does a workaround of adding a fake unreachable
branch that doesn't jump away, for each place this is a problem.
  • Loading branch information
jemc committed May 30, 2024
1 parent 4e3eddc commit a7693ed
Show file tree
Hide file tree
Showing 6 changed files with 221 additions and 189 deletions.
1 change: 1 addition & 0 deletions spec/File.Result.Spec.savi
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
:it "describes without using 2nd person language"
File.Result.each_enum_member -> (member |
assert: !member.description.includes("you")
assert: !member.description.includes("You")
)

:it "describes without any un-escaped newline characters in the text"
Expand Down
76 changes: 39 additions & 37 deletions src/File.Dumper.savi
Original file line number Diff line number Diff line change
Expand Up @@ -23,45 +23,47 @@
callback File.Dumper.ResultActor = File.Dumper.ResultActor.None
)
try (
_Unsafe.check_path!(path.string) -> (error |
callback.result_of_dump_to_file(path, error)
)
_Unsafe.check_path!(path.string)
| error |
callback.result_of_dump_to_file(path, error)
return
)

fd = _Unsafe.open_write_create!(path) -> (error |
case error == (
| File.Result.PathPrefixNotFound |
// TODO: Create the prefix directories instead of being asinine.
None
| File.Result.AccessDenied |
if Platform.is_windows (
// TODO: Check if the path refers to a directory. If it is, switch
// the error to File.Result.PathIsDirectory, which is more helpful.
)
| File.Result.NeedsRetry |
@dump_to_file(path, chunks, callback) // try again
return
fd = try (
_Unsafe.open_write_create!(path)
| error |
case error == (
| File.Result.PathPrefixNotFound |
// TODO: Create the prefix directories instead of being asinine.
None
| File.Result.AccessDenied |
if Platform.is_windows (
// TODO: Check if the path refers to a directory. If it is, switch
// the error to File.Result.PathIsDirectory, which is more helpful.
)

callback.result_of_dump_to_file(path, error)
| File.Result.NeedsRetry |
@dump_to_file(path, chunks, callback) // try again
return
)

try (
_Unsafe.writev_loop!(fd, chunks) -> (error |
callback.result_of_dump_to_file(path, error)
)
_Unsafe.fsync!(fd) -> (error |
callback.result_of_dump_to_file(path, error)
)
_Unsafe.close!(fd) -> (error |
callback.result_of_dump_to_file(path, error)
)
callback.result_of_dump_to_file(path, File.Result.Success)
|
// If any of the above steps failed, just close the file descriptor.
_Unsafe.close!(fd) -> (error |
// This is the failure path - we already called a callback with an
// error above, so we don't want to forward an additional error here,
// even if the close step failed with an error.
)
)
callback.result_of_dump_to_file(path, error)
return
)

try (
_Unsafe.writev_loop!(fd, chunks)
_Unsafe.fsync!(fd)
_Unsafe.close!(fd)

callback.result_of_dump_to_file(path, File.Result.Success)
| error |
callback.result_of_dump_to_file(path, error)

// If any of the above steps failed, just close the file descriptor.
//
// If this close raises an error we'll swallow it, because we are
// already in the failure path - we already called a callback with an
// error above, so we don't want to forward an additional error here,
// even if the close step failed with an error.
try _Unsafe.close!(fd)
)
4 changes: 1 addition & 3 deletions src/File.Info.savi
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,4 @@
:: Raises an error if the file does not exist or could not be accessed
:: (for any reason).
:fun non for!(path File.Path.Readable) File.Info
_Unsafe.stat_as_file_info!(path) -> (error |
// Silently ignore the specific kind of error - we won't show the caller.
)
_Unsafe.stat_as_file_info!(path)
68 changes: 35 additions & 33 deletions src/File.Loader.savi
Original file line number Diff line number Diff line change
Expand Up @@ -23,42 +23,44 @@
callback File.Loader.ResultActor
)
try (
_Unsafe.check_path!(path.string) -> (error |
callback.result_of_load_from_file(path, error, Bytes.new_iso)
)
_Unsafe.check_path!(path.string)
| error |
callback.result_of_load_from_file(path, error, Bytes.new_iso)
return
)

fd = _Unsafe.open_read!(path) -> (error |
case error == (
| File.Result.AccessDenied |
if Platform.is_windows (
// TODO: Check if the path refers to a directory. If it is, switch
// the error to File.Result.PathIsDirectory, which is more helpful.
)
| File.Result.NeedsRetry |
@load_from_file(path, callback) // try again
return
fd = try (
_Unsafe.open_read!(path)
| error |
case error == (
| File.Result.AccessDenied |
if Platform.is_windows (
// TODO: Check if the path refers to a directory. If it is, switch
// the error to File.Result.PathIsDirectory, which is more helpful.
)

callback.result_of_load_from_file(path, error, Bytes.new_iso)
| File.Result.NeedsRetry |
@load_from_file(path, callback) // try again
return
)
callback.result_of_load_from_file(path, error, Bytes.new_iso)
return
)

try (
buffer = Bytes.new
_Unsafe.read_all!(path, fd, buffer) -> (error |
callback.result_of_load_from_file(path, error, buffer.take_buffer)
)
buffer = Bytes.new
result = File.Result.Success
try (
_Unsafe.read_all!(path, fd, buffer)
| error |
result = error
)

// Success!
result = File.Result.Success
callback.result_of_load_from_file(path, result, buffer.take_buffer)
)
// Close the file descriptor prior to exit (on success or failure).
//
// Note that we don't deal with the error here, because it seems like
// there's not much we can do in the case of failure.
//
// At any rate, this is only a read operation, so it won't leave
// the filesystem in an inconsistent state. It seems unlikely to matter.
try _Unsafe.close!(fd)

// Close the file descriptor prior to exit (on success or failure).
_Unsafe.close!(fd) -> (error |
// Note that we don't deal with the error here, because it seems like
// there's not much we can do in the case of failure.
//
// At any rate, this is only a read operation, so it won't leave
// the filesystem in an inconsistent state. It seems unlikely to matter.
)
)
callback.result_of_load_from_file(path, result, buffer.take_buffer)
26 changes: 24 additions & 2 deletions src/File.Result.savi
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,15 @@
:member TooManyFilesOpenInSystem 405
:member TooManySymbolicLinksFollowed 406
:member TooManyFilesOpenInProcess 407
:member LowLevelIOFailed 408
:member LowLevelCorruption 409
:member TooManyWatchesOpenInSystem 408
:member LowLevelIOFailed 409
:member LowLevelCorruption 410

///
// Fundamental problems with the system where the program is running.

:member PlatformDoesNotSupportFiles 501
:member PlatformDoesNotSupportWatching 502

:fun description String
case @ == (
Expand Down Expand Up @@ -155,6 +162,11 @@
perhaps it needs to impose explicit limits on itself to prevent this. \
It may be possible to increase the limit if more files are needed."

| File.Result.TooManyWatchesOpenInSystem |
"There are too many watched file paths currently in the system. \
This may be caused by a user-level limit or system-level limit. \
It may be possible to increase the limit if more watches are needed."

| File.Result.LowLevelIOFailed |
"A low-level I/O failure occurred while synchronizing state with the \
underlying file. It may (or may not) help to retry the operation."
Expand All @@ -163,6 +175,16 @@
"A low-level corruption while interacting with the file system \
prevented success. It may (or may not) help to retry the operation."

| File.Result.PlatformDoesNotSupportFiles |
"The platform where the program is running does not support accessing \
files at all. The program is likely running on a sandboxed platform \
(such as WebAssembly in the browser)."

| File.Result.PlatformDoesNotSupportWatching |
"The platform where the program is running does not support watching \
watching files/directories. The only platforms with usable watching APIs \
are MacOS, Windows, Linux, and FreeBSD."

|
// This should be unreachable, if we've covered all the enum members.
// We have a test in File.Result.Spec which proves that we did.
Expand Down
Loading

0 comments on commit a7693ed

Please sign in to comment.