Skip to content

refactor: convert more array-building loops to for-comprehensions#3447

Closed
bobzhang wants to merge 1 commit intomainfrom
hongbo/comprehension-batch-3
Closed

refactor: convert more array-building loops to for-comprehensions#3447
bobzhang wants to merge 1 commit intomainfrom
hongbo/comprehension-batch-3

Conversation

@bobzhang
Copy link
Copy Markdown
Contributor

@bobzhang bobzhang commented Apr 20, 2026

Summary

Fourth in the series (after #3444, #3445, #3446). Aggressive sweep targeting sites where the comprehension form is clearly more concise, even when it drops an explicit `capacity` pre-alloc hint:

  • `argparse/parser_positionals.mbt`: `positional_args`
  • `hashset/hashset.mbt`: `HashSet::to_array` (previously kept out of refactor: apply for-comprehension to more array-building loops #3446 to preserve capacity hint — now included)
  • `immut/sorted_set/immutable_set.mbt`: `ToJson` impl
  • `list/list.mbt`: `ToJson` impl
  • `sorted_map/deprecated.mbt`: `keys`, `values` (uses two-binding `for k, _ in self` / `for _, v in self`)
  • `sorted_map/map.mbt`: `SortedMap::to_array`

Discovered limitation

Array comprehensions reject calling functions that can raise ("calling function with error is not allowed inside list comprehension"). This blocks conversion of `Array::filter`, `Array::filter_map`, `ArrayView::filter`, `ArrayView::filter_map`, etc., which take `(T) -> Bool raise?` predicates. Those are left untouched.

Test plan

  • `moon check` clean
  • `moon test -p argparse -p list -p hashset -p sorted_map -p immut/sorted_set` passes (916 tests)
  • `moon fmt --check` clean
  • `codex review --uncommitted` confirms behavior-preserving

🤖 Generated with Claude Code


Open in Devin Review

Third follow-up to #3444, applying the pattern more aggressively:

- argparse/parser_positionals.mbt: positional_args
- hashset/hashset.mbt: HashSet::to_array (drops explicit capacity hint)
- immut/sorted_set/immutable_set.mbt: ToJson impl
- list/list.mbt: ToJson impl
- sorted_map/deprecated.mbt: keys, values (uses Iter2 two-binding for)
- sorted_map/map.mbt: to_array

Array comprehensions do not allow calling functions that can raise, so
Array::filter, Array::filter_map, and their ArrayView counterparts are
left untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coveralls
Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 3876

Coverage decreased (-0.004%) to 94.91%

Details

  • Coverage decreased (-0.004%) from the base build.
  • Patch coverage: 5 of 5 lines across 5 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 15540
Covered Lines: 14749
Line Coverage: 94.91%
Coverage Strength: 220112.48 hits per line

💛 - Coveralls

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@bobzhang
Copy link
Copy Markdown
Contributor Author

Superseded by #3448 which consolidates all comprehension refactors.

@bobzhang bobzhang closed this Apr 21, 2026
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