with_ascii_lowercased zig builtin #7496
Conversation
HajagosNorbert
force-pushed
the
ascii-builtins
branch
from
9f102a0 to
1266186
Compare
|
CI was failing on the roc formatter check and on the use of camelCased |
HajagosNorbert
force-pushed
the
ascii-builtins
branch
from
1266186 to
8b016f0
Compare
|
This is the same issue as in #7514, I'll be looking at it today |
Thanks Anton! |
crates/compiler/builtins/roc/Str.roc
Outdated
| ## expect "CAFÉ".with_ascii_lowercased() == "cafÉ" | ||
| ## ``` | ||
| ## | ||
| ## This function is useful for things like [command-line options](https://en.wikipedia.org/wiki/Command-line_interface#Command-line_option) and [environment variables](https://en.wikipedia.org/wiki/Environment_variablewhere you ## know in advance that you're dealing with a hardcoded string containing only ASCII characters. It has better performance than lowercasing operations which take Unicode into account. |
There was a problem hiding this comment.
These comments are missing some newlines.
| assert_evals_to!( | ||
| r#" | ||
| original = "cOFFÉ cOFFÉ cOFFÉ cOFFÉ cOFFÉ cOFFÉ cOFFÉ cOFFÉ cOFFÉ cOFFÉ cOFFÉ cOFFÉ" | ||
| res = Str.with_ascii_lowercased original |
There was a problem hiding this comment.
Consider using PNC instead of WSA to avoid needing to update these tests in the near future
| @@ -374,7 +375,12 @@ pub const RocStr = extern struct { | |||
| return 1; | |||
| } | |||
|
|
|||
| const ptr: [*]usize = @as([*]usize, @ptrCast(@alignCast(self.bytes))); | |||
| const data_ptr = if (self.isSeamlessSlice()) | |||
There was a problem hiding this comment.
We check self.isSeamlessSlice() twice in this function. It's not an expensive operation per se, but it's multiple operations. Can we maybe avoid checking it multiple times by storing in a var or something?
|
@HajagosNorbert to get the fix from PR #7532 you just need to pull main, execute the mono tests and |
|
Thanks @Anton-4, I'm waiting for all the tests to pass locally, after including the changes from Sam's review. |
HajagosNorbert
force-pushed
the
ascii-builtins
branch
from
8b016f0 to
f372e18
Compare
Please, before you approve, check out the zig code changes, since I'm unsure about handling reference counts. Tests were passing whether I put a referece count decrement statement in the with_ascii_lowercased function or not for the non-unique strings branch, so the
string.decref();is in there quiet arbitrarily.initFromBigStrisn't used anywhere, so I deleted it.The test
with_ascii_lowercased_non_zero_refcounttestincrates/compiler/test_gen/src/gen_str.rsis an attempt at testing the function when the refcount of it's input string is > 1, but I'm not sure if that's what it does actually.