Skip to content

Conversation

@Inkrementator
Copy link
Contributor

VariantN structs contain a buffer to store the payload. The size of that
buffer is determined by a template argument and thus part of the type.
If you try to store a bigger payload in the same variable, the buffer
size can't change, so what is done instead is that Variant will store a
pointer to the data on the heap.

The problem is that now we have to update this pointer on every copy
operation, otherwise Variant switches from Value semantics to reference
semantics.

This can either be done in the actual postblit of variant or the handler
function on the operation OpID.postblit. I chose the handler, because it
access to the type of the payload, the Variant postblit function would
have dispatch to the handler every time otherwise to get the typeinfo
and thus the size of the payload at runtime.

Fixes #10062
Fixes #9783

Bug #9783 had the same root cause. The reduction from that issue became the testcase for this and has been modified it to hit all branches in the OpID.postblit handling.

VariantN structs contain a buffer to store the payload. The size of that
buffer is determined by a template argument and thus part of the type.
If you try to store a bigger payload in the same variable, the buffer
size can't change, so what is done instead is that Variant will store a
pointer to the data on the heap.

The problem is that now we have to update this pointer on every copy
operation, otherwise Variant switches from Value semantics to reference
semantics.

This can either be done in the actual postblit of variant or the handler
function on the operation OpID.postblit. I chose the handler, because it
access to the type of the payload, the Variant postblit function would
have dispatch to the handler every time otherwise to get the typeinfo
and thus the size of the payload at runtime.

Fixes dlang#10062
Fixes dlang#9783

Bug dlang#9783 had the same root cause. The reduction from that issue became
the testcase for this and has been modified it to hit all branches in
the OpID.postblit handling.
std.variant didn't compile when the payload was a struct that was bigger than
its internal buffer and had to be allocated on the heap if that struct
`@disabled this()`. The new handling now always skips the constructor and just
copies the directly to the allocated regions.
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @Inkrementator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

  • In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#10925"

The condition that lead to postblit creation contained a typo which
sadly didn't trigger a compiler error. Fix that and also verify in
unittests that postblit is only generated when necessary.

handler(OpID.postblit) would've probably still be a no-op, so this is
technically just a performance optimization.
Only generate handler for OpID.postblit if it is possible for it to be
called.
No need to generate it for each different VariantN instantiation.
@thewilsonator thewilsonator merged commit 7c37289 into dlang:master Jan 10, 2026
9 of 10 checks passed
@thewilsonator
Copy link
Contributor

Does this fix #9783 ?

@Inkrementator
Copy link
Contributor Author

Yes

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

std.variant does not observe value semantics for large value types. std.variant doesn't do postblit/dtor correctly for large structs

4 participants