[WIP] Fix memset, memcpy, memmove calls on Pointer type. #4611
Conversation
src/pointer.cr
Outdated
| size.to_u32 | ||
| end | ||
| {% end %} | ||
|
|
There was a problem hiding this comment.
No. Intrinsics method signatures are not associated to LibC::SizeT.
This PR is short one, without changing of rules of the game.
There was a problem hiding this comment.
No idea what you're talking about. Why spray the definition of size_t all over the codebase when it already exists.
Use LibC::SizeT.new instead of to_size_t and you don't need this method.
There was a problem hiding this comment.
Yes, we need to introduce generic SizeT and OffsetT types, or change Intrinsics contracts, but it should be discussed and introduced by another PR.
This PR just fixes contract implementation.
akzhan
force-pushed
the
fix-pointer-contract
branch
from
88c2200
to
73c3c4b
Compare
Just note that it's short fix for crystal-lang#4589 only.
akzhan
force-pushed
the
fix-pointer-contract
branch
from
73c3c4b
to
0417f80
Compare
| @@ -242,7 +242,7 @@ struct Pointer(T) | |||
| raise ArgumentError.new("Negative count") if count < 0 | |||
|
|
|||
| if self.class == source.class | |||
| Intrinsics.memcpy(self.as(Void*), source.as(Void*), (count * sizeof(T)).to_u32, 0_u32, false) | |||
| Intrinsics.memcpy(self.as(Void*), source.as(Void*), count * sizeof(T), 0_u32, false) | |||
There was a problem hiding this comment.
I have no idea why the CI isn't broken but isn't this passing 32bit ints to something expecting a U64?
|
Dislike this as it actually returns a different type depending on the architecture. I'd prefer a SizeT. |
akzhan
changed the title
|
This is a palliative. Will rework using SizeT and OffsetT. |
|
Sorry, that doesn't solve anything. It still relies on |
|
So it should be |
|
@konovod thanks. was fixed with #4624. @ysbaddaden thanks too. |
Just note that it's short fix for #4589 only.
No generic SizeT, OffsetT introduced here.