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

Expand standard library #339

Merged
merged 1 commit into from
Dec 8, 2023
Merged

Expand standard library #339

merged 1 commit into from
Dec 8, 2023

Conversation

jiribenes
Copy link
Contributor

@jiribenes jiribenes commented Dec 4, 2023

Motivation

I'm doing Advent of Code 2023 and wanted to upstream a few useful functions :)

Changes

In general, I tried to reconcile the different versions, conventions, and limitations of all of the different backends... 🤯
It would be really nice to have a solution for #309 -- some of the functions would work on any backend and I wouldn't have to copy them over 5 times.

Newly added functions by module:

effekt

all backends

  • min(n: Int, m: Int): Int, no specific tests, upstreamed from LLVM backend
  • max(n: Int, m: Int): Int, no specific tests, upstreamed from LLVM backend

immutable/list

all backends

  • foreachIndex[A](l: List[A]) { f: (Int, A) => Unit } : Unit, no specific tests
  • collect[A, B](l: List[A]) { f : A => Option[B] }: List[B] with new tests in examples/pos/list/collect
  • flatMap[A, B](l: List[A]) { f : A => List[B] }: List[B] with new tests in examples/pos/list/flatMap
  • sum(l: List[Int]): Int with new tests in examples/pos/list/sum

text/string

not in LLVM backend

  • indexOf(str: String, sub: String): Option[Int] with new tests in examples/pos/string/indexOf; not in the Chez backend
  • lastIndexOf(str: String, sub: String): Option[Int] with new tests in examples/pos/string/indexOf; not in the Chez backend
  • startsWith(str: String, prefix: String): Boolean and endsWith(str: String, suffix: String): Boolean, no specific tests; not in the Chez backend
  • substring(str: String, from: Int, to: Int): String with new tests in examples/pos/string/substring
    • most backends had substring(str: String, from: Int): String with many different semantics
    • currently claps from and to to be between 0 and str.length
    • ... and makes sure that if from >= to, then the result is ""
  • ANSI escape codes (ANSI_GREEN, ANSI_RED, ANSI_RESET)

mutable/array

only in the JS backend

  • reformatted the whole module to be more consistent with the rest of the codebase
  • foreach[T](arr: Array[T]){ action: T => Unit / Control }: Unit
  • foreachIndex[T](arr: Array[T]){ action: (Int, T) => Unit / Control }: Unit
  • sum(list: Array[Int]): Int with new tests in examples/pos/array/sum
  • there's also new big set of tests for going between Array[T] and List[T] in examples/pos/array/list_conversion to prevent problems like in JS backend: Array.toList drops first element #319
  • BREAKING renamed: arrayFromValue→fill, arrayFromLoop→build, arrayFromList→fromList

mutable/map

only in the JS backend

  • reformatted the whole module to be more consistent with the rest of the codebase
  • added keys, values which return a JS array from mutable/array -- there are no tests for this

test

  • ported from ML backend to JS and Chez with its tests
  • could/should be extracted out to shared stdlib (Share standard library code between backends #309)
  • added assertEqual -- this could possibly fail on backends without general equality, but it's still very useful to me
    • uses an explicit show block argument because of backends that don't have a show function for all types (related: Typeclasses #66)

@jiribenes jiribenes changed the title [WIP] Expand standard library Expand standard library Dec 5, 2023
@jiribenes jiribenes marked this pull request as ready for review December 5, 2023 14:09
jiribenes added a commit that referenced this pull request Dec 6, 2023
BREAKING CHANGES:
- mutable/array: renamed:
    - arrayFromValue → fill
    - arrayFromLoop → build
    - arrayFromList → fromList

Changes by module:

[effekt]

_all backends_

- `min(n: Int, m: Int): Int`, no specific tests, upstreamed from LLVM backend
- `max(n: Int, m: Int): Int`, no specific tests, upstreamed from LLVM backend

[immutable/list]

_all backends_

- `foreachIndex[A](l: List[A]) { f: (Int, A) => Unit } : Unit`, no specific tests
- `collect[A, B](l: List[A]) { f : A => Option[B] }: List[B]` with new tests in `examples/pos/list/collect`
- `flatMap[A, B](l: List[A]) { f : A => List[B] }: List[B]` with new tests in `examples/pos/list/flatMap`
- `sum(l: List[Int]): Int` with new tests in `examples/pos/list/sum`

[text/string]

_not in LLVM backend_

- `indexOf(str: String, sub: String): Option[Int]` with new tests in `examples/pos/string/indexOf`; not in the Chez backend
- `lastIndexOf(str: String, sub: String): Option[Int]` with new tests in `examples/pos/string/indexOf`; not in the Chez backend
- `startsWith(str: String, prefix: String): Boolean` and `endsWith(str: String, suffix: String): Boolean`, no specific tests; not in the Chez backend
- `substring(str: String, from: Int, to: Int): String` with new tests in `examples/pos/string/substring`
  - most backends had `substring(str: String, from: Int): String` with many different semantics
  - currently claps `from` and `to` to be between `0` and `str.length`
  - ... and makes sure that if `from >= to`, then the result is `""`
- ANSI escape codes (`ANSI_GREEN`, `ANSI_RED`, `ANSI_RESET`)

[mutable/array]

_only in the JS backend_

- reformatted the whole module to be more consistent with the rest of the codebase
- `foreach[T](arr: Array[T]){ action: T => Unit / Control }: Unit`
- `foreachIndex[T](arr: Array[T]){ action: (Int, T) => Unit / Control }: Unit`
- `sum(list: Array[Int]): Int` with new tests in  `examples/pos/array/sum`
- there's also new big set of tests for going between `Array[T]` and `List[T]` in `examples/pos/array/list_conversion` to prevent problems like in #319
- **BREAKING** renamed: `arrayFromValue→fill`, `arrayFromLoop→build`, `arrayFromList→fromList`

[mutable/map]

_only in the JS backend_

- reformatted the whole module to be more consistent with the rest of the codebase
- added `keys`, `values` which return a JS array from `mutable/array` -- there are no tests for this

[test]

- ported from ML backend to JS and Chez with its tests
- could/should be extracted out to shared stdlib (#309)
- added `assertEqual` -- this could possibly fail on backends without general equality, but it's still very useful to me
  - uses an explicit `show` block argument because of backends that don't have a `show` function for all types (related: #66)

---

This is a combination of 33 commits:

immutable/list: add collect, flatMap, sum
mutable/array: reformat
mutable/array: add foreach and foreachIndex
mutable/array: add sum
text/string: add ANSI escape codes
text/string: add indexOf, lastIndexOf
mutable/array: disable array tests on ML backend
immutable/list: Add foreachIndex, sum, collect, flatMap to ML backend
text/string: add indexOf, lastIndexOf to ML backend
text/string: fix 'indexOf' tests for backends without universal print
text/string: rename 'index_of' tests to 'indexOf' to reflect fn name
immutable/list: try adding foreachIndex, collect, flatMap, sum to Chez backend(s)
text/string: ignore 'indexOf' tests in the Chez backend
immutable/list: fix sum in Chez backend (oops)
text/string: add startsWith, endsWith to JS backend
text/string: use proper parameter name in endsWith in ML backend
text/string: change behaviour of lastIndexOf on ML backend to align with JS native impl
mutable/array: don't run list conversion tests on Chez backend
text/string: add tests for substring
effekt.effekt: add min, max
text/string: attempt to unify 'substring' behaviour across backends
text/string: fix off-by-one error in substring (oops)
mutable/array: ignore 'sum' test on Chez backend(s)
text/string: fix 'substring' failing on last character (oops)
text/string: fix thinko in substring on ML backend
text/string: clarify semantics of substring, add more tests
text/string: add ANSI escape sequences to Chez backend(s)
test: port testing framework from ML backend
immutable/list: try ading foreachIndex, collect, flatMap, sum to LLVM backend
test: pass an explicit 'show' block to 'assertEqual' ML backend
immutable/list: explicitly import immutable/option in 'collect' test
mutable/array: rename: arrayFromValue→fill, arrayFromLoop→build, arrayFromList→fromList
mutable/map: add 'values' and 'keys' to JS-backed map
@jiribenes jiribenes force-pushed the feature/expand-stdlib-aoc2023 branch from 91da20a to 1de0665 Compare December 6, 2023 10:29
@jiribenes
Copy link
Contributor Author

Squashed the 33 commits into a single large commit with a huge description :)

@jiribenes
Copy link
Contributor Author

Please note the breaking changes in mutable/array: renamed:

  • arrayFromValue → fill
  • arrayFromLoop → build
  • arrayFromList → fromList

A [untested!] command like sed -i '' -e 's/arrayFromValue/fill/g' -e 's/arrayFromLoop/build/g' -e 's/arrayFromList/fromList/g' some-effekt-file.effekt should be enough for migration.

cc @IR0NSIGHT

BREAKING CHANGES:
- mutable/array: renamed:
    - arrayFromValue → fill
    - arrayFromLoop → build
    - arrayFromList → fromList

Changes by module:

[effekt]

_all backends_

- `min(n: Int, m: Int): Int`, no specific tests, upstreamed from LLVM backend
- `max(n: Int, m: Int): Int`, no specific tests, upstreamed from LLVM backend

[immutable/list]

_all backends_

- `foreachIndex[A](l: List[A]) { f: (Int, A) => Unit } : Unit`, no specific tests
- `collect[A, B](l: List[A]) { f : A => Option[B] }: List[B]` with new tests in `examples/pos/list/collect`
- `flatMap[A, B](l: List[A]) { f : A => List[B] }: List[B]` with new tests in `examples/pos/list/flatMap`
- `sum(l: List[Int]): Int` with new tests in `examples/pos/list/sum`

[text/string]

_not in LLVM backend_

- `indexOf(str: String, sub: String): Option[Int]` with new tests in `examples/pos/string/indexOf`; not in the Chez backend
- `lastIndexOf(str: String, sub: String): Option[Int]` with new tests in `examples/pos/string/indexOf`; not in the Chez backend
- `startsWith(str: String, prefix: String): Boolean` and `endsWith(str: String, suffix: String): Boolean`, no specific tests; not in the Chez backend
- `substring(str: String, from: Int, to: Int): String` with new tests in `examples/pos/string/substring`
  - most backends had `substring(str: String, from: Int): String` with many different semantics
  - currently claps `from` and `to` to be between `0` and `str.length`
  - ... and makes sure that if `from >= to`, then the result is `""`
- ANSI escape codes (`ANSI_GREEN`, `ANSI_RED`, `ANSI_RESET`)

[mutable/array]

_only in the JS backend_

- reformatted the whole module to be more consistent with the rest of the codebase
- `foreach[T](arr: Array[T]){ action: T => Unit / Control }: Unit`
- `foreachIndex[T](arr: Array[T]){ action: (Int, T) => Unit / Control }: Unit`
- `sum(list: Array[Int]): Int` with new tests in  `examples/pos/array/sum`
- there's also new big set of tests for going between `Array[T]` and `List[T]` in `examples/pos/array/list_conversion` to prevent problems like in #319
- **BREAKING** renamed: `arrayFromValue→fill`, `arrayFromLoop→build`, `arrayFromList→fromList`

[mutable/map]

_only in the JS backend_

- reformatted the whole module to be more consistent with the rest of the codebase
- added `keys`, `values` which return a JS array from `mutable/array` -- there are no tests for this

[test]

- ported from ML backend to JS and Chez with its tests
- could/should be extracted out to shared stdlib (#309)
- added `assertEqual` -- this could possibly fail on backends without general equality, but it's still very useful to me
  - uses an explicit `show` block argument because of backends that don't have a `show` function for all types (related: #66)

---

This is a combination of 33 commits:

immutable/list: add collect, flatMap, sum
mutable/array: reformat
mutable/array: add foreach and foreachIndex
mutable/array: add sum
text/string: add ANSI escape codes
text/string: add indexOf, lastIndexOf
mutable/array: disable array tests on ML backend
immutable/list: Add foreachIndex, sum, collect, flatMap to ML backend
text/string: add indexOf, lastIndexOf to ML backend
text/string: fix 'indexOf' tests for backends without universal print
text/string: rename 'index_of' tests to 'indexOf' to reflect fn name
immutable/list: try adding foreachIndex, collect, flatMap, sum to Chez backend(s)
text/string: ignore 'indexOf' tests in the Chez backend
immutable/list: fix sum in Chez backend (oops)
text/string: add startsWith, endsWith to JS backend
text/string: use proper parameter name in endsWith in ML backend
text/string: change behaviour of lastIndexOf on ML backend to align with JS native impl
mutable/array: don't run list conversion tests on Chez backend
text/string: add tests for substring
effekt.effekt: add min, max
text/string: attempt to unify 'substring' behaviour across backends
text/string: fix off-by-one error in substring (oops)
mutable/array: ignore 'sum' test on Chez backend(s)
text/string: fix 'substring' failing on last character (oops)
text/string: fix thinko in substring on ML backend
text/string: clarify semantics of substring, add more tests
text/string: add ANSI escape sequences to Chez backend(s)
test: port testing framework from ML backend
immutable/list: try ading foreachIndex, collect, flatMap, sum to LLVM backend
test: pass an explicit 'show' block to 'assertEqual' ML backend
immutable/list: explicitly import immutable/option in 'collect' test
mutable/array: rename: arrayFromValue→fill, arrayFromLoop→build, arrayFromList→fromList
mutable/map: add 'values' and 'keys' to JS-backed map
@jiribenes jiribenes force-pushed the feature/expand-stdlib-aoc2023 branch from 1de0665 to 32be17b Compare December 6, 2023 10:42
@jiribenes
Copy link
Contributor Author

Rebased on current master (be63dbb).

@@ -22,6 +22,45 @@ def foreach[A](l: List[A]) { f: A => Unit } : Unit =
foreach(rest) { a => f(a) }
}

def foreachIndex[A](l: List[A]) { f: (Int, A) => Unit } : Unit = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Most other languages that I am aware of first list the element, then the index. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No preference from my point of view, I just followed the order which was already present in immutable/list on the JS backend 😇

https://github.com/effekt-lang/effekt/blob/master/libraries/js/immutable/list.effekt#L23

Copy link
Collaborator

Choose a reason for hiding this comment

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

well... what did I think back then? Let's merge first, then share, then reorganize.

Copy link
Contributor Author

@jiribenes jiribenes Dec 8, 2023

Choose a reason for hiding this comment

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

Most other languages that I am aware of first list the element, then the index. WDYT?

I took a quick look at other languages:

Python with enumerate:

for index, elem in enumerate(myIterator):

Go with range:

for index, elem := range mySlice { ... }

C++20 with enumerate:

for (auto const [index, elem] : std::views::enumerate(v))

Kotlin:

fun <T> Iterable<T>.forEachIndexed(
    action: (index: Int, T) -> Unit)
)

Haskell (vector library):

iforM :: Monad m => Vector a -> (Int -> a -> m b) -> m (Vector b)

Koka:

fun foreach-indexed(xs: list<a>, action: (int, a) -> e ()): e ()

... but:

  • Scala uses zipWithIndex and has the (T, Int) order instead
  • JavaScript uses .forEach((element, index) => ...)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Funny, I looked at JS, Scala, Ruby...

@b-studios
Copy link
Collaborator

Thanks @jiribenes this is fantastic. But you are right and we really need to start sharing things after merging this.

Another thing that I have in my mind is that map and collect both could be implemented in terms of Control to allow breaking out and skipping elements.

But that is something for future work.

Copy link
Collaborator

@b-studios b-studios left a comment

Choose a reason for hiding this comment

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

LGTM the only bikeshed I have is the order of arguments on foreachIndex

@b-studios b-studios merged commit 62c266c into master Dec 8, 2023
2 checks passed
@b-studios b-studios deleted the feature/expand-stdlib-aoc2023 branch December 8, 2023 12:40
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.

None yet

2 participants