Skip to content

refactor: apply for-comprehension to more array-building loops#3446

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

refactor: apply for-comprehension to more array-building loops#3446
bobzhang wants to merge 1 commit intomainfrom
hongbo/comprehension-batch-2

Conversation

@bobzhang
Copy link
Copy Markdown
Contributor

@bobzhang bobzhang commented Apr 20, 2026

Summary

Third in the series (after #3444 and #3445). Converts five more imperative push-loops to for-comprehensions:

  • `argparse/parser.mbt`: `group_usage_expr` (uses `is Some(idx)` binding)
  • `argparse/parser_lookup.mbt`: `split_long`
  • `env/env_native.mbt`: `get_cli_args_internal`
  • `hashset/hashset.mbt`: `ToJson` impl (uses `is Some({ key, .. })` binding)
  • `set/linked_hash_set.mbt`: `ToJson` impl

Skipped sites:

  • `HashSet::to_array` — hot-path, keeps explicit `capacity=self.size` hint.
  • `parser_globals_merge` two-loop concat — needs spread/extend, not a clean comprehension.
  • `positional_usage`, `option_entries`, `arg_display` — branching pushes; not idiomatic to unify.

Test plan

  • `moon check` clean
  • `moon test -p argparse -p env -p hashset -p set` passes (700 tests)
  • `moon fmt --check` clean
  • `codex review --uncommitted` confirms behavior-preserving across wasm/wasm-gc/js/native targets

🤖 Generated with Claude Code


Open in Devin Review

Second follow-up to #3444, extending the same pattern to:

- argparse/parser.mbt: group_usage_expr
- argparse/parser_lookup.mbt: split_long
- env/env_native.mbt: get_cli_args_internal
- hashset/hashset.mbt: ToJson impl
- set/linked_hash_set.mbt: ToJson impl

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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

@coveralls
Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 3875

Coverage increased (+0.004%) to 94.918%

Details

  • Coverage increased (+0.004%) from the base build.
  • Patch coverage: 6 of 6 lines across 4 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: 15545
Covered Lines: 14755
Line Coverage: 94.92%
Coverage Strength: 220041.8 hits per line

💛 - Coveralls

@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