Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

return Result<T, E> from IO functions #407

Merged
merged 17 commits into from
Aug 6, 2024
Merged

return Result<T, E> from IO functions #407

merged 17 commits into from
Aug 6, 2024

Conversation

enricozb
Copy link
Contributor

@enricozb enricozb commented Aug 1, 2024

No description provided.

enricozb added 6 commits July 24, 2024 16:41
@enricozb
return Result from IO functions
44ed8f1
@enricozb
(cuda attempt) return Result from IO functions
909d468
@enricozb
fix WORK mode not executing high-priority redexes
2777433 
because of the original form of this if-statement, threads that were in
WORK mode and that had a high-priority redex but no low priority redexes
would not do any work. this led to a bug where `save_redexes` was called
while high-priority redexes were still present.
@enricozb
remove redundant new_port(.., get_val(..))
ed069c8
@enricozb
(cuda) return result from io functions
1e5b9f5 
returns Result<T,E> for appropriate io functions, matches up with C
implementation
@enricozb
remove redundant new_port(.., get_val(..))
d029c7f
@enricozb enricozb requested a review from developedby August 1, 2024 14:09
developedby
developedby requested changes Aug 1, 2024
View reviewed changes
Copy link
Member

@developedby developedby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to also update hvm.h

src/hvm.c Show resolved Hide resolved
src/run.c Show resolved Hide resolved
src/run.c Outdated Show resolved Hide resolved
src/run.c Outdated Show resolved Hide resolved
src/run.c Outdated Show resolved Hide resolved
enricozb added 8 commits August 5, 2024 11:33
@enricozb
return Result<T, IOError<E>> from IO functions
f93b294 
Since some IO errors are shared across IO functions (invalid argument
type, unexpected IO function name), we add a custom IOError ADT. Now,
every IO function (even those that cannot fail) will return a
Result<T, IOError<E>>. When attempting to invoke an IO function with an
invalid name, an IOError/Name is returned.
@enricozb
additional IO tests, along with their Bend implementations
aa9f15c 
Add io programs that test basic functionality, including failures.
This also updates the "smoke test" file, that does a sequence of
open, read, write, close io calls on a file.
@enricozb
fix resource allocation failure print
bc66920
@enricozb
reduce num nodes in basic.bend::main
352e2e2 
basic.bend couldn't be run in CUDA as the main definition was too large
@enricozb
align cuda with c
be6ad6f
@enricozb
add test-io to io files
09b314d 
otherwise the rust impl will run on these
@enricozb
update demo_io files
b802f0b 
they were stale, and didn't represent the shortest bend code that can
invoke io functions
@enricozb
add test snapshots
9c58be2
@enricozb enricozb requested a review from developedby August 5, 2024 10:48
developedby
developedby requested changes Aug 5, 2024
View reviewed changes
Copy link
Member

@developedby developedby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to update the lib headers hvm.h and hvm.cuh

examples/demo_io/main.bend Outdated Show resolved Hide resolved
@developedby developedby mentioned this pull request Aug 5, 2024
Closed
enricozb added 2 commits August 5, 2024 19:09
@enricozb
read LICENSE instead of README.md in tests
6506a98 
the README is more likely to change and break tests than the LICENSE
file.
@enricozb
update .h and .cuh interfaces to reflect Str changes
7846b50 
the Str struct was stale in the header files
@enricozb enricozb requested a review from developedby August 5, 2024 17:33
@enricozb enricozb marked this pull request as ready for review August 5, 2024 17:34
developedby
developedby approved these changes Aug 5, 2024
View reviewed changes
Copy link
Member

@developedby developedby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks alright

@enricozb
Copy link
Contributor Author

enricozb commented Aug 6, 2024 •
edited
Loading

Going to hold off on merging as there is some issue with the C runtime hanging on this code (on an M1 mac only, not on my intel machine)

def unwrap(res):
  match res:
    case Result/Ok:
      return res.val
    case Result/Err:
      return res.val

(Main) =
  with IO {
    ask res = (call "foo" (1, 2))

    (unwrap res)
  }

@developedby developedby mentioned this pull request Aug 6, 2024
Closed
@enricozb
check ctr sizes in do_run_io
888b2a4 
amazingly this bug hadn't been caught before. we were not checking the
number of arguments in `ctr` before accessing them, therefore accessing
meaningless memory regions, which could lead to infinite loops as
nothing would be reduced and the same "interaction" would be attempted
endlessly.
@enricozb enricozb merged commit a5e6788 into main Aug 6, 2024
3 of 4 checks passed
@enricozb enricozb mentioned this pull request Aug 7, 2024
Closed
@developedby developedby mentioned this pull request Aug 20, 2024
Closed
2 tasks
@enricozb enricozb mentioned this pull request Aug 20, 2024
Open
6 tasks
@kings177 kings177 added this to the HVM IO lib v0 milestone Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@developedby developedby developedby approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
HVM IO lib v0
Development

Successfully merging this pull request may close these issues.
3 participants
@enricozb @developedby @kings177