Skip to content

Conversation

PatrickHaecker
Copy link
Contributor

@PatrickHaecker PatrickHaecker commented Feb 23, 2025

Fixes the double accounting of the union byte array in Base.summarysize as described in #57506.

If this is the correct fix, can it be backported to 1.11?

Fix #57506

@PatrickHaecker
Copy link
Contributor Author

This is only one possible fix. The alternative would be to not account for the union byte array in sizeof. This depends on the exact definition of sizeof which is discussed in #54007.

It might be that there is a more consistent way to define sizeof. If this is done, the implementation of Base.summarysize might need to be adapted. However, the current result of Base.summarysize is just wrong and should therefore be fixed.

@giordano giordano changed the title Fix #57506: Base.summarysize for Memory with Union Base.summarysize for Memory with Union Feb 23, 2025
@LilithHafner LilithHafner added the bugfix This change fixes an existing bug label Feb 23, 2025
Copy link
Member

@LilithHafner LilithHafner left a comment

Choose a reason for hiding this comment

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

Thanks for contributing, @PatrickHaecker! This looks great to me. I'm going to wait a bit before merging to see of @oscardssmith wants to chime in.

@oscardssmith oscardssmith merged commit 7b7ba33 into JuliaLang:master Feb 23, 2025
10 checks passed
@oscardssmith
Copy link
Member

thanks for this!

@nsajko nsajko added backport 1.11 Change should be backported to release-1.11 backport 1.12 Change should be backported to release-1.12 labels Feb 24, 2025
KristofferC pushed a commit that referenced this pull request Feb 26, 2025
Fixes the double accounting of the union byte array in
`Base.summarysize` as described in #57506.

If this is the correct fix, can it be backported to 1.11?

Fix #57506

(cherry picked from commit 7b7ba33)
KristofferC pushed a commit that referenced this pull request Mar 11, 2025
Fixes the double accounting of the union byte array in
`Base.summarysize` as described in #57506.

If this is the correct fix, can it be backported to 1.11?

Fix #57506

(cherry picked from commit 7b7ba33)
@KristofferC KristofferC mentioned this pull request Mar 11, 2025
71 tasks
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Mar 24, 2025
KristofferC pushed a commit that referenced this pull request Mar 25, 2025
Fixes the double accounting of the union byte array in
`Base.summarysize` as described in #57506.

If this is the correct fix, can it be backported to 1.11?

Fix #57506

(cherry picked from commit 7b7ba33)
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This change fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Base.summarysize incorrect for Memory containing Union

5 participants