Skip to content

Support nogc_construct of struct with disabled postblit.#24

Merged
LunaTheFoxgirl merged 2 commits intoInochi2D:mainfrom
p0nce:placement-new-disabled-struct
Dec 4, 2025
Merged

Support nogc_construct of struct with disabled postblit.#24
LunaTheFoxgirl merged 2 commits intoInochi2D:mainfrom
p0nce:placement-new-disabled-struct

Conversation

@p0nce
Copy link
Copy Markdown
Contributor

@p0nce p0nce commented Dec 3, 2025

This one is a bit tricky.

The issue it fix is "not being able to nogc_construct a disabled-postblit struct on DMDFE >= 2111" (eg: LDC 1.41.0)

Here the placement new will act directly on the referenced object (instead of relying on declaring a temp initialized T and assign).

HOWEVER

Calling emplace on uninitialized memory is considered UB by numem, but it does visibly happen in numem and the code I replaced was doing a .init initialization with T tmp;.
So out of desire to not break anything, initializeAt(dst); was added.

I checked in an uncommited test that placement new does NOT initialize memory.

I think "nogc_construct" contract should read:

  • calling nogc_construct on uninitialized object is UB" (though for now it works)
  • calling nogc_construct on created object is illegal
    (EDIT: tests pass locally but yeah probably trickier than that)

Didn't work in the new placement new path.
@p0nce
Copy link
Copy Markdown
Contributor Author

p0nce commented Dec 3, 2025

Can repro CI failure with DMD 2.111

@LunaTheFoxgirl LunaTheFoxgirl merged commit f48ab14 into Inochi2D:main Dec 4, 2025
39 checks passed
@p0nce
Copy link
Copy Markdown
Contributor Author

p0nce commented Dec 4, 2025

Sorry to ask, a git tag would one thing Dplug needs to depend on numem, since today all objects numem does all object creation/destruction in ~main. :)

@LunaTheFoxgirl
Copy link
Copy Markdown
Member

Sure, will create a new tag in a bit.

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.

2 participants