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.

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

Updates builtin type Map so that it stores Maybes in its nodes. #743

Merged
merged 2 commits into from
Dec 26, 2024

Conversation

In-Veritas
Copy link
Contributor

@In-Veritas In-Veritas commented Dec 7, 2024

Summary of Changes

The following updates have been made to builtins.bend:

New/Updated Types and Functions

1. Maybe

  • New type definition:
    type Maybe(T):
      Some {val: T}
      None
    

2. Map

  • Updated type definition:

    type Map(T):
      Node {value: Maybe(T), ~left: Map(T), ~right: Map(T)}
      Leaf
    

    Now, every Node stores a Maybe type to explicitly represent the presence or absence of a value.

  • Updated/added functions:

    • Maybe/unwrap: Retrieves the value inside Maybe/Some
    • Map/set: Inserts or updates a value in the map.
    • Map/get: Retrieves a value from the map, returning a Maybe type.
    • Map/map: Applies a function to each value in the map.
    • Map/contains: Checks whether a key exists in the map.
    • Map/get_check: Retrieves a value and performs additional validation.

Why remove unreachable() from Map nodes?

  • You cannot see which elements are in the Map, nor can you perform a fold operation on the Map to, for example, return a list of values or map each value. Since the stored elements are not usable for anything, it is not possible to use this structure to determine whether a given element is present in the map or not.
  • The unreachable() function incorrectly ignores type check and can often cause bugs where the type isn't checked for an entire branch altogether.
  • When checking the results of a program, receiving * does not tell you whether you merely hit an unused node, or if another function called unreachable().

Tests

  • Tests for the new functions have been added at Bend/tests/golden_tests/prelude

Documentation update

Partially updates documentation adding a section for the Maybe type and new Map functions

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.

Why are you changing the Map type? You didn't even mention that in the PR.

Also please update the PR name to something more succint, its a bit confusing.

I'm not against having Bool in the prelude, but we don't really need it for anything builtin.

tests/builtin_Map_tests.bend Outdated Show resolved Hide resolved
examples/example_fun.bend Outdated Show resolved Hide resolved
examples/parallel_and.bend Outdated Show resolved Hide resolved
src/fun/builtins.bend Show resolved Hide resolved
src/fun/builtins.bend Show resolved Hide resolved
tests/snapshots/run_file__nested_map_get.bend.snap Outdated Show resolved Hide resolved
tests/snapshots/run_file__nested_map_set.bend.snap Outdated Show resolved Hide resolved
tests/snapshots/run_file__unbound_wrap.bend.snap Outdated Show resolved Hide resolved
@In-Veritas In-Veritas changed the title Updates builtin type Map, and its corresponding functions to the imp syntax, and adds types Bool and Maybe to builtins Updates builtin type Map so that it stores Maybes in its notes. Dec 9, 2024
@In-Veritas In-Veritas changed the title Updates builtin type Map so that it stores Maybes in its notes. Updates builtin type Map so that it stores Maybes in its nodes. Dec 9, 2024
In-Veritas added a commit to In-Veritas/Bend that referenced this pull request Dec 16, 2024
examples/parallel_and.bend Outdated Show resolved Hide resolved
src/fun/builtins.bend Outdated Show resolved Hide resolved
return (unreachable(), map)

# Checks if a node has a value on a given key, returning Maybe/Some if it does, Maybe/None otherwise
def Map/get_check (map: Map(T), key: u24) -> (Map(T), Maybe(T)):
Copy link
Member

Choose a reason for hiding this comment

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

get_check is not a great name. I prefer get_checked but it's also not great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_node?

Copy link
Member

Choose a reason for hiding this comment

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

get also gets the node

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_wrapped?

src/fun/builtins.bend Outdated Show resolved Hide resolved
src/fun/builtins.bend Outdated Show resolved Hide resolved
tests/golden_tests/run_file/mixed_syntax.bend Outdated Show resolved Hide resolved
tests/golden_tests/run_file/unscoped_never_used.bend Outdated Show resolved Hide resolved
tests/snapshots/run_file__nested_map_get.bend.snap Outdated Show resolved Hide resolved
tests/snapshots/run_file__nested_map_set.bend.snap Outdated Show resolved Hide resolved
tests/golden_tests.rs Outdated Show resolved Hide resolved
return (unreachable(), map)

# Checks if a node has a value on a given key, returning Maybe/Some if it does, Maybe/None otherwise
def Map/get_check (map: Map(T), key: u24) -> (Map(T), Maybe(T)):
Copy link
Member

Choose a reason for hiding this comment

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

get also gets the node

tests/golden_tests/prelude/map_contains_test.bend Outdated Show resolved Hide resolved
docs/builtins.md Outdated Show resolved Hide resolved
docs/builtins.md Outdated Show resolved Hide resolved
examples/example_fun.bend Outdated Show resolved Hide resolved
tests/snapshots/prelude__builtin_Map_tests.bend.snap Outdated Show resolved Hide resolved
tests/snapshots/prelude__get_values_from_map.bend.snap Outdated Show resolved Hide resolved
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.

I'll trust the tests are correct

tests/snapshots/prelude__map_tests.bend.snap Outdated Show resolved Hide resolved
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 good, good job

@developedby
Copy link
Member

I think a new Rust version is messing with the checks, I'll fix it don't worry

@developedby
Copy link
Member

Can you rebase and try again?
It should be fixed on the current main branch

…as the Maybe type

Updates builtin type Map so that it stores Maybes in its nodes.

Implement builtin type Map, and its corresponding functions, as well as the Maybe type

Updates builtin type Map so that it stores Maybes in its nodes.

Changes made on  09-12-2024, includes fixing requests from HigherOrderCO#743

Fixes based on PR review

Partially updates doc and follows requests from PR review

Fixes documentation, makes tests return simpler values, deletes useless snapshot

Removes map_tests.bend.snap
@developedby developedby added this pull request to the merge queue Dec 26, 2024
Merged via the queue into HigherOrderCO:main with commit a52cce7 Dec 26, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants