Skip to content

refactor(beam): give string x and x.ToString() one dispatch#4816

Merged
dbrattli merged 1 commit into
mainfrom
refactor/beam-tostring-dispatch
Jul 18, 2026
Merged

refactor(beam): give string x and x.ToString() one dispatch#4816
dbrattli merged 1 commit into
mainfrom
refactor/beam-tostring-dispatch

Conversation

@dbrattli

Copy link
Copy Markdown
Collaborator

Summary

The to-string dispatch was duplicated byte-for-byte between two replacement tables, differing only in whether the type variable was spelled _t or t:

operators, | "ToString", [ arg ] reached by string x
conversions, | "ToString", _ reached by x.ToString()

Both branch on arg.Type through the same six cases (String, Char, Decimal, the floats, other numerics, Boolean, and a generic fallback) and emit the same code for each.

Why it is worth removing

Nothing marked the two as copies, so a fix applied to one spelling and not the other stays invisible until someone happens to test the neglected one — string x correct and x.ToString() wrong, or the reverse.

#4814 is the near miss. It had to add the same boxed-char peel to both sites, with the same five-line rationale paraphrased twice:

// `box c |> string` arrives as a char under a boxing cast to `obj`, and would otherwise fall
// through to `fable_convert:to_string`, which sees only the integer and prints the codepoint.
let arg = Chars.tryAsChar arg |> Option.defaultValue arg

It got both, but only because the duplication happened to be noticed.

What changed

Extracted to ToString.toStringByType in Beam/Prelude.fs, next to the Chars helpers it depends on, with the peel folded in and its rationale stated once. Both call sites become one line each.

Testing

./build.sh test beam green: .NET 2614 passed / 0 failed, Erlang 2633 PASS / 0 FAIL, entry-point program tests passed.

No new tests, because there is no new behaviour to cover. What is checked instead is that there is none: all 82 generated Erlang files under temp/tests/Beam/src are byte-identical before and after the change (diff -r), so nothing about the emitted code moved.

🤖 Generated with Claude Code

The 14-line to-string dispatch was duplicated byte-for-byte between the
`operators` and `conversions` tables, differing only in whether the type
variable was spelled `_t` or `t`. Both handle `ToString`; they are just
reached by different routes.

Nothing warned that there were two copies, so a fix applied to one
spelling and not the other would be invisible until someone happened to
test the neglected one. #4814 is the near miss: it had to add the same
boxed-char peel, with the same rationale, to both — and did, but only
because the duplication was noticed.

Now `ToString.toStringByType` in Beam/Prelude.fs, next to the `Chars`
helpers it uses, with the peel folded in and the rationale stated once.

Pure refactor: all 82 generated Erlang files under temp/tests/Beam/src
are byte-identical before and after.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@dbrattli
dbrattli merged commit cc61a44 into main Jul 18, 2026
31 checks passed
@dbrattli
dbrattli deleted the refactor/beam-tostring-dispatch branch July 18, 2026 14:12
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.

1 participant