Skip to content

Add Enum.parse(Bytes) and Enum.parse?(Bytes) - #17175

Open
jgaskins wants to merge 4 commits into
crystal-lang:masterfrom
jgaskins:enum-parse-bytes
Open

Add Enum.parse(Bytes) and Enum.parse?(Bytes)#17175
jgaskins wants to merge 4 commits into
crystal-lang:masterfrom
jgaskins:enum-parse-bytes

Conversation

@jgaskins

Copy link
Copy Markdown
Contributor

Fixes #17174

Comment thread src/enum.cr Outdated
slice.each do |byte|
next if byte == '-'.ord || byte == '_'.ord
byte_counter += 1
return nil if byte_counter > {{max_size}}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

issue: We cannot compare a byte index with a char size.
As mentioned above, there is no StringLiteral#bytesize equivalent. So keeping this optimization is a bit tricky. Perhaps we could calculate the max bytesize at runtime?
But it should be fine to just use max_size * 4 as limit, same as for the buffer size.

@jgaskins jgaskins Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point. I didn't think about enum members with non-ASCII names. There weren't any tests for them, so I added some, including a couple with case-folding, in e3a80b8.

A discussion for another time is whether we should add support for Bytes to Char::Reader. Or maybe some different construct that's allocated on the stack since Char::Reader is a reference type and we're trying to avoid heap allocations. I'm not sure how to articulate my thoughts on this, but the summary is that I've now implemented parsing bytes into UTF-8 chars in two different PRs (here and in #17065) and that feels like we need a better abstraction.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Char::Reader is a struct with value semantics

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, I must've gotten it confused with something else. We can disregard that part, then.

Comment thread src/enum.cr Outdated
jgaskins added 2 commits July 29, 2026 23:34
Using strings, we had support for that out of the box, but we had to
recreate it for byte-parsing.
Comment thread spec/std/enum_spec.cr Outdated
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
@ysbaddaden

ysbaddaden commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

While the feature is nice, it also duplicates the whole Unicode parsing that is already implemented for String 😢

Maybe we should re-think the current String methods that actually operate on Bytes internally. There could be an abstraction layer to iterate every Char in Bytes (Unicode aware), to parse "123" into an Int32, ...

The string.each_char(&) could become Unicode.each_char(bytes, &) for example, and that would be the whole change for this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Enum.parse(Bytes) and Enum.parse?(Bytes)

4 participants