Skip to content

Conversation

Asterless
Copy link
Contributor

@Asterless Asterless commented Jun 10, 2025

Summary

This PR enhances error handling across various functions in the codebase, allowing for proper error propagation in higher-order functions.

Details

Affected Functions

  • Immutable Array

    • each
    • eachi
    • makei
    • fold
    • fold_left and fold_right
    • rev_fold
    • map
  • Immutable HashMap

    • each
    • filter
    • fold and fold_with_key
    • intersection_with
    • map and map_with_key
    • union_with (updated to propagate errors from merging function)
  • Immutable HashSet

    • each
  • Legacy Immutable List

    • all
    • any
    • drop_while
    • each and eachi
    • filter
    • find and findi
    • flat_map
    • fold and foldi and rev_fold and rev_foldi
    • fold_left and fold_lefti
    • fold_right and fold_righti
    • mapi and rev_map
    • scan_left and scan_right
    • unfold
  • Immutable Sorted Map

    • filter and filter_with_key
  • Immutable SortedSet

    • all
    • any
    • each
    • filter
    • fold
    • map
  • Built-in Iteration

    • Iter::each
    • Iter::eachi
    • Iter::fold

These updates ensure a more robust and consistent approach to error handling across multiple modules, including array operations, immutable data structures, and iteration utilities.

Copy link

peter-jerry-ye-code-review bot commented Jun 10, 2025

Consistent error propagation signature change across collection types

Category
Correctness
Code Snippet
fn[K : Compare, V] filter(self : T[K, V], pred : (V) -> Bool) -> T[K, V]
// Changed to:
fn[K : Compare, V] filter(self : T[K, V], pred : (V) -> Bool raise?) -> T[K, V] raise?
Recommendation
Add documentation to explain error handling behavior in the public API documentation
Reasoning
The change makes higher-order functions error-aware by propagating potential errors from callback functions. Users need to understand when and how errors might be propagated through collection operations.

Impact on backwards compatibility with existing code

Category
Maintainability
Code Snippet
fn[T, B] Iter::fold(Self[T], init~ : B, (B, T) -> B) -> B
// Changed to:
fn[T, B] Iter::fold(Self[T], init~ : B, (B, T) -> B raise?) -> B raise?
Recommendation
Consider providing non-raising variants of common operations to maintain backwards compatibility
Reasoning
Existing code using these functions may not expect or handle errors. Having both raising and non-raising variants would ease migration.

Consistent application of `raise?` annotations

Category
Maintainability
Code Snippet
fn[A, B] map(self : T[A], (A) -> B) -> T[B]
// Some implementations changed to raise? while others remain unchanged
Recommendation
Review and align error handling behavior across all higher-order functions consistently
Reasoning
Some higher-order functions retained their non-raising signatures while similar functions were changed to support error propagation. This inconsistency could lead to confusion.

@coveralls
Copy link
Collaborator

coveralls commented Jun 10, 2025

Pull Request Test Coverage Report for Build 7308

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.103%

Totals Coverage Status
Change from base Build 7304: 0.0%
Covered Lines: 8572
Relevant Lines: 9207

💛 - Coveralls

@peter-jerry-ye peter-jerry-ye marked this pull request as draft June 11, 2025 03:53
@peter-jerry-ye peter-jerry-ye force-pushed the Refactor/@Immut-ErrorPolymorphism branch 2 times, most recently from ec7e49c to 13e58f3 Compare June 16, 2025 03:23
@peter-jerry-ye peter-jerry-ye marked this pull request as ready for review June 16, 2025 03:24
@peter-jerry-ye peter-jerry-ye force-pushed the Refactor/@Immut-ErrorPolymorphism branch from 13e58f3 to 1a81258 Compare June 16, 2025 03:24
@peter-jerry-ye peter-jerry-ye enabled auto-merge (rebase) June 16, 2025 03:33
@peter-jerry-ye peter-jerry-ye disabled auto-merge June 16, 2025 03:33
@peter-jerry-ye peter-jerry-ye enabled auto-merge (squash) June 16, 2025 03:34
@peter-jerry-ye
Copy link
Collaborator

@Asterless next time, please check the content generated by the Copilot before you submit a PR.

@peter-jerry-ye peter-jerry-ye merged commit a76bb55 into moonbitlang:main Jun 16, 2025
10 of 12 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.

4 participants