Skip to content

Serialization stdlib: Replace a call to Core.memoryref with Core.memoryrefnew#61137

Merged
oscardssmith merged 2 commits intomasterfrom
dpa/serialization-memoryref-method
Mar 3, 2026
Merged

Serialization stdlib: Replace a call to Core.memoryref with Core.memoryrefnew#61137
oscardssmith merged 2 commits intomasterfrom
dpa/serialization-memoryref-method

Conversation

@DilumAluthge
Copy link
Member

The Serialization stdlib has the following function:

function deserialize(s::AbstractSerializer, X::Type{MemoryRef{T}} where T)
x = Core.memoryref(deserialize(s))::X
i = deserialize(s)::Int
i == 2 || (x = Core.memoryref(x, i, true))
return x::X
end

As far as I can tell, there is no method matching Core.memoryref(x::MemoryRef, i::Int, true::Bool) (called on line 1448).1

I think this should be a call to Core.memoryrefnew(x::MemoryRef, i::Int, true::Bool) instead, which is what this PR does.

Footnotes

  1. This issue was detected by JET.

@oscardssmith
Copy link
Member

good catch. There was a name change post this code getting written.

@oscardssmith oscardssmith added bugfix This change fixes an existing bug merge me PR is reviewed. Merge when all tests are passing labels Feb 24, 2026
@adienes
Copy link
Member

adienes commented Feb 24, 2026

a test could be good

@IanButterworth IanButterworth added needs tests Unit tests are required for this change and removed merge me PR is reviewed. Merge when all tests are passing labels Feb 25, 2026
@nsajko nsajko added the stdlib Julia's standard library label Feb 26, 2026
@DilumAluthge
Copy link
Member Author

DilumAluthge commented Feb 28, 2026

Agreed, but I have no idea how to write a test for this. Any ideas?

EDIT: I've added some tests.

@DilumAluthge DilumAluthge force-pushed the dpa/serialization-memoryref-method branch from fb9ac44 to d4cf899 Compare February 28, 2026 21:02
@DilumAluthge DilumAluthge force-pushed the dpa/serialization-memoryref-method branch from d4cf899 to 8ac44d6 Compare February 28, 2026 21:31
@DilumAluthge DilumAluthge added backport 1.13 Change should be backported to release-1.13 and removed backport 1.13 Change should be backported to release-1.13 labels Feb 28, 2026
@DilumAluthge
Copy link
Member Author

The call to Core.memoryref(x, i, true) exists in this line on release-1.11, release-1.12, and release-1.13. It does not exist in release-1.10.

@DilumAluthge
Copy link
Member Author

@oscardssmith Does this need to be backported to 1.11, 1.12, and 1.13?

@oscardssmith oscardssmith added backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 backport 1.13 Change should be backported to release-1.13 labels Feb 28, 2026
@oscardssmith
Copy link
Member

Looks like this does need backports. This was missed in #54681

@DilumAluthge
Copy link
Member Author

I've added some tests. @oscardssmith Can you give this another review?

@DilumAluthge DilumAluthge removed the needs tests Unit tests are required for this change label Mar 1, 2026
@KristofferC KristofferC mentioned this pull request Mar 3, 2026
56 tasks
@oscardssmith oscardssmith merged commit acd9708 into master Mar 3, 2026
8 of 9 checks passed
@oscardssmith oscardssmith deleted the dpa/serialization-memoryref-method branch March 3, 2026 16:48
KristofferC pushed a commit that referenced this pull request Mar 5, 2026
…emoryrefnew` (#61137)

The Serialization stdlib has the following function:

https://github.com/JuliaLang/julia/blob/e46125f569f43e28261d1b55a0d85aacceee499a/stdlib/Serialization/src/Serialization.jl#L1445-L1450

As far as I can tell, there is no method matching
`Core.memoryref(x::MemoryRef, i::Int, true::Bool)` (called on line
1448).[^1]

[^1]: This issue was detected by JET.

I think this should be a call to `Core.memoryrefnew(x::MemoryRef,
i::Int, true::Bool)` instead, which is what this PR does.

(cherry picked from commit acd9708)
KristofferC pushed a commit that referenced this pull request Mar 5, 2026
…emoryrefnew` (#61137)

The Serialization stdlib has the following function:

https://github.com/JuliaLang/julia/blob/e46125f569f43e28261d1b55a0d85aacceee499a/stdlib/Serialization/src/Serialization.jl#L1445-L1450

As far as I can tell, there is no method matching
`Core.memoryref(x::MemoryRef, i::Int, true::Bool)` (called on line
1448).[^1]

[^1]: This issue was detected by JET.

I think this should be a call to `Core.memoryrefnew(x::MemoryRef,
i::Int, true::Bool)` instead, which is what this PR does.

(cherry picked from commit acd9708)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 backport 1.13 Change should be backported to release-1.13 bugfix This change fixes an existing bug stdlib Julia's standard library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants