Add __delete for migration from delete #2037

wilzbach · 2018-01-11T18:25:05Z

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.

JinShil · 2018-01-11T23:12:48Z

changelog/core-memory-__delete.dd

+
+$(CONSOLE
+sed "s/delete \(.*\);/__delete(\1);/" -i **/*.d
+)


I suggest adding something to the effect that this is intended to only be used as a last resort when destroy is not an option. At least that's my understanding: We want to migrate users to destroy.

Potential verbage: "Users should prefer object.destroy to explicitly finalize objects, and only resort to __delete when object.destroy would not be a feasible option."

The sed command could be a bit refined: s/\Wdelete[ \t]+\(.*\);/__delete(\1);/g

JinShil · 2018-01-11T23:13:36Z

src/core/memory.d

+This function has been added to provide an easy transition from `delete`.
+It performs the same functionality as `delete`.
+
+$(REF destroy, object) is `@safe` and should be used if possible.


Again, I think this should have some information explaining that users should first look to destroy and only to __delete when destroy is not an option.

JinShil · 2018-01-11T23:16:55Z

src/foo.d

@@ -0,0 +1,151 @@
+import std.stdio;


What is this file (src/foo.d) doing in this commit?

ibuclaw · 2018-01-12T09:22:17Z

Looks like you committed an extra file by mistake

ibuclaw · 2018-01-12T09:23:08Z

What advantages does this have over the (poorly named) destroy?

JinShil · 2018-01-12T09:28:11Z

What advantages does this have over the (poorly named) destroy?

My understanding is it was mandated by @andralex. See discussion starting here

ibuclaw · 2018-01-12T09:35:58Z

Still, my understanding was that (years ago when delete was first put for deprecation) destroy was added as a migration path - by Andrei himself I believe.

Obviously that function has failed spectacularly if this is needed. 🙄

wilzbach · 2018-01-12T14:19:33Z

Still, my understanding was that (years ago when delete was first put for deprecation) destroy was added as a migration path - by Andrei himself I believe.
Obviously that function has failed spectacularly if this is needed. 🙄

A bit of history I could dug up, but it seems that destroy has been there for a long time:

#231
#7

JinShil · 2018-01-12T14:33:27Z

src/core/memory.d

+        memory.
+        If the garbage collector was not used to allocate the memory for
+        the instance, undefined behavior will result.
+    )


The implementation just sets the pointer to null. I don't think that initiates an immediate call to the GC to free the memory.

JinShil · 2018-01-12T14:34:36Z

src/core/memory.d

+    a.test = "exists";
+    auto b = &a;
+    auto c = &b;
+    delete c;


Shouldn't this be __delete?

JinShil · 2018-01-12T14:36:39Z

src/core/memory.d

+    auto b = a;
+    __delete(b);
+    assert(b is null);
+    assert(a == [1, 2, 3]);


If I'm understanding correctly, you are accessing through a after b was __deleteed. According to the documentation comments, that's undefined behavior.

JinShil · 2018-01-12T14:44:24Z

src/core/memory.d

+    }
+    else
+    {
+        static assert(0, "It is not possible to delete: " ~ T.stringof);


"It is not possible to delete`" ~ T.stringof ~ "`"

ibuclaw · 2018-01-12T19:48:38Z

A bit of history I could dug up, but it seems that destroy has been there for a long time:

231 is titled "Rename clear to destroy". So it could always be my memory that is incorrect. :-)

jmdavis · 2018-01-13T03:45:36Z

clear was talked about in TDPL. I don't know how old it is, but I don't think that it was new then. Regardless, it's never been about freeing memory, only about destroying the object and putting it in an invalid state, so it's really not the same as what this is trying to do - though in principle, the idea behind clear/destroy and getting rid of delete is that no one should ever do what delete does if they're using GC-allocated memory. Andrei has been adamant about that being a terrible idea for years now, but we've just never gotten rid of delete (if nothing else, because without allocators, trying to replace code that uses with code that does something similar with non-GC-allocated memory it is a royal pain).

I am a bit surprised thought that Andrei wants this delete-replacement. From what I recall, his stance in the past has always been that delete is fundamentally a bad idea for GC-allocated memory and that it should be gone, and keeping a library function around that does the same thing doesn't fit with that - though the fact that it's not built-in does reduce its visibility and make it seem less acceptable / more dangerous, so I guess that he considers that enough now.

Personally, I think that anyone looking to do something like this should not be using GC-allocated memory, but it's not like we're completely against letting folks risk blowing their own feet off - we just don't want it to be easy to do that by default.

andralex · 2018-01-13T04:07:24Z

changelog/core-memory-__delete.dd

+`core.memory.__delete` has been added
+
+$(REF __delete, core, memory) allows easy migration from the deprecated `delete`.
+`__delete` behaves exactly like `delete` and will call the class dtor immediately


delete/__delete apply to structs and arrays as well, so please update the documentation

andralex · 2018-01-13T04:10:32Z

changelog/core-memory-__delete.dd

+
+$(CONSOLE
+sed "s/delete \(.*\);/__delete(\1);/" -i **/*.d
+)


The sed command could be a bit refined: s/\Wdelete[ \t]+\(.*\);/__delete(\1);/g

andralex · 2018-01-13T04:13:11Z

src/core/memory.d

+    }
+    else static if (is(T == struct))
+    {
+        _d_delstruct(cast(void**) &t, typeid(T));


Wait, this is incorrect. __delete should be only applicable to pointers to struct. Let's not innovate here.

andralex · 2018-01-13T04:13:23Z

src/core/memory.d

+    }
+    else static if (is(T == U*, U))
+    {
+        t = null;


this should free memory

andralex · 2018-01-13T04:13:31Z

src/core/memory.d

+    }
+    else static if (is(T : E[], E))
+    {
+        t = null;


this should free memory

andralex · 2018-01-13T04:16:02Z

Still, my understanding was that (years ago when delete was first put for deprecation) destroy was added as a migration path - by Andrei himself I believe.

Obviously that function has failed spectacularly if this is needed. 🙄

Thanks for the quip. The olden idea is to keep the safer clear while still allowing people to free objects. That was (and is) possible by means of two function calls. More recently our doctrine for deprecation has become to provide a simple replacement for people who have delete spread out through their code and just want to keep the current semantics for the time being. So both clear and __delete have their place.

wilzbach · 2018-01-13T09:39:19Z

Alrighty I went over the implementation again and it now matches with DMD 1:1 -> https://github.com/dlang/dmd/blob/v2.078.0/src/dmd/e2ir.d#L3886

The only thing that I couldn't figure out is how to detect at compile-time whether -profile=gc has been set by the user (DMD uses a different function mapping for this option - see toTraceGC), but I doubt that it's essential for this PR as -profile=gdc is mostly about detecting allocations.

JinShil · 2018-01-13T14:58:23Z

changelog/core-memory-__delete.dd

+`core.memory.__delete` has been added
+
+$(REF __delete, core, memory) allows easy migration from the deprecated `delete`.
+`__delete` behaves exactly like $(LINK2 $(ROOT_DIR)spec/expression.html#delete_expressions, `delete`):


One of the procedures we need to perform to deprecate and eventually remove delete is to remove all references to it in the spec (see dlang/dlang.org#1941). I suggest removing the link to the spec.

andralex · 2018-01-13T15:36:48Z

src/core/memory.d

+        there is a destructor for that class, the destructor
+        is called for that object instance.
+    )
+    $(LI Otherwise, the garbage collector is called to immediately free the


This "otherwise" is ambiguous because it's applied to a conjunction. Does it refer to the fact that t is a class object reference, to the fact there is a destructor, or to the conjunction? (It applies to the second clause only). I'll make an edit to clarify.

FYI: That was taken 1:1 from the spec ;-)

@wilzbach @WalterBright and I are at POPL 2018 and one thing that became abundantly clear is we must overhaul the spec making it radically more precise. More on that later.

andralex · 2018-01-13T16:12:21Z

@wilzbach I just sent a patch to the doc - we should reuse destroy both in the documentation and the implementation. Also it seems associative arrays have been forgotten.

wilzbach · 2018-01-13T16:38:56Z

src/core/memory.d

+$(UL
+    $(LI
+        Calls `.object.destroy(*x)` (if `x` is a pointer) or `.object.destroy(x)`
+        (otherwise) to destroy the referred entity.


That's strictly speaking not true for class/interface objects and raw memory:

Exact behavior

If x is a class or interface object -> rt_finalize

druntime/src/rt/lifetime.d

Lines 163 to 167 in 098a08a

else

{

rt_finalize(cast(void*) *p);

}

GC.free(cast(void*) *p);

Of course, object.destroy does call rt_finalize:

druntime/src/object.d

Lines 2887 to 2896 in 098a08a

void destroy(T)(T obj) if (is(T == class))

{

rt_finalize(cast(void*)obj);

}

/// ditto

void destroy(T)(T obj) if (is(T == interface))

{

destroy(cast(Object)obj);

}

FTR rt_finalize is deprecated, but rt_finalize2 does call a few interesting things:

druntime/src/rt/lifetime.d

Lines 1382 to 1419 in 098a08a

debug(PRINTF) printf("rt_finalize2(p = %p)\n", p);

auto ppv = cast(void**) p;

if(!p || !*ppv)

return;

auto pc = cast(ClassInfo*) *ppv;

try

{

if (det || collectHandler is null || collectHandler(cast(Object) p))

{

auto c = *pc;

do

{

if (c.destructor)

(cast(fp_t) c.destructor)(cast(Object) p); // call destructor

}

while ((c = c.base) !is null);

}

if (ppv[1]) // if monitor is not null

_d_monitordelete(cast(Object) p, det);

if (resetMemory)

{

auto w = (*pc).initializer;

p[0 .. w.length] = w[];

}

}

catch (Exception e)

{

import core.exception : onFinalizeError;

onFinalizeError(*pc, e);

}

finally

{

*ppv = null; // zero vptr even if `resetMemory` is false

}

If x is a reference to a struct -> Typeinfo_Struct.destroy:

druntime/src/rt/lifetime.d

Lines 177 to 183 in 098a08a

extern (C) void _d_delstruct(void** p, TypeInfo_Struct inf)

{

if (*p)

{

debug(PRINTF) printf("_d_delstruct(%p, %p)\n", *p, cast(void*)inf);

inf.destroy(*p);

Here it's all about calling the dtor:

druntime/src/object.d

Lines 1194 to 1199 in 098a08a

if (xdtor)

{

if (m_flags & StructFlags.isDynamicType)

(*xdtorti)(p, this);

else

(*xdtor)(p);

If x is a reference to an arrays -> for all structs within: Typeinfo_Struct.destroy:

druntime/src/rt/lifetime.d

Line 1169 in 098a08a

finalize_array(start, length, ti);

druntime/src/rt/lifetime.d

Lines 1347 to 1356 in 098a08a

void finalize_array(void* p, size_t size, const TypeInfo_Struct si)

{

// Due to the fact that the delete operator calls destructors

// for arrays from the last element to the first, we maintain

// compatibility here by doing the same.

auto tsize = si.tsize;

for (auto curP = p + size - tsize; curP >= p; curP -= tsize)

{

// call destructor

si.destroy(curP);

Otherwise only GC.free is called.

wilzbach · 2018-01-13T16:39:43Z

src/core/memory.d

+        Frees the memory allocated for `x`. If `x` is a reference to a class
+        or interface, the memory allocated for the underlying instance is freed. If `x` is
+        a pointer, the memory allocated for the pointed-to object is freed. If `x` is a
+        built-in array or associative array, the memory allocated for the array is freed.


Associative arrays aren't allowed: https://run.dlang.io/is/CtfP8f

wilzbach · 2018-01-13T16:40:29Z

src/core/memory.d

+)
+
+Params:
+    t = aggregate object that should be destroyed


Reminder to myself: I will rename this to x then.

schveiguy · 2018-01-13T16:49:38Z

From my memory:

clear/GC.free was intended as the replacement for delete. The problem with delete is that it wasn't possible to destroy things deterministically without also creating a dangling pointer. The required conflation of memory management with finalization/destruction was a faulty mechanism, especially in a GC-by-default language. I think the intention was to allow a library function, and stop reserving a keyword for such a thing.
clear was renamed to destroy because .clear is typically the name people use for removing all elements of a container. Given how prevalent UFCS is in D, then this creates massive confusion.

IMO, the destroy/GC.free mechanism is too raw, however. Sometimes it's exactly what you want, and GC.free requires a pointer, destroy requires a value. You are using clunky mechanisms everywhere if you use this thing.

wilzbach · 2018-01-13T16:52:47Z

@wilzbach I just sent a patch to the doc

Nice (I just removed your trailing white-space in case you were wondering why I force-pushed your commit).

andralex · 2018-01-13T17:16:51Z

@wilzbach yah I wonder if we could submit a github issue - would be great if their editor supported trailing space elimination

andralex · 2018-01-13T17:18:58Z

@wilzbach agreed about the discrepancies - in this case we must work backwards: __delete IS (by fiat) "destroy then deallocate". To the extent it isn't we need to adjust either destroy or __delete.

wilzbach · 2018-01-14T22:53:32Z

yah I wonder if we could submit a github issue - would be great if their editor supported trailing space elimination

https://github.com/isaacs/github

@wilzbach agreed about the discrepancies - in this case we must work backwards: __delete IS (by fiat) "destroy then deallocate". To the extent it isn't we need to adjust either destroy or __delete.

What do you suggest? Do you want to change __delete do call destroy?
_d_delclass still checks for deallocators as class deallocators haven't been deprecated yet.

I would simply change the wording to sth. like:

Calls .object.destroy(x) (if x is a class or interface object) or typeid(*x).destroy(x) (if x is pointer to a struct) to destroy the referred entity. Arrays of structs call typeid(*x).destroy(x) for each element.

andralex · 2018-01-14T22:56:16Z

Yes, __delete must call destroy. The wording you propose is good, thanks.

wilzbach · 2018-01-14T23:28:28Z

src/core/memory.d

+    $(LI
+        Calls `.object.destroy(x)` (if `x` is a class or interface object) or
+        `typeid(*x).destroy(x)` (if `x` is pointer to a struct) to destroy the referred entity.
+        Arrays of structs call `typeid(*x).destroy(x)` for each element.


This could also state: Arrays of structs call typeid(x).destroy(&x) for each element (that's what the implementation does below).

I think it should be .object.destroy(e) for each element e in the array.

wilzbach · 2018-01-14T23:30:17Z

src/core/memory.d

+    assert(a is null);
+}
+
+// __delete returns with no effect if x is null


Is there a simple way to add a test this for "no effect"?

andralex · 2018-01-14T23:51:05Z

src/core/memory.d

+$(UL
+    $(LI
+        Calls `.object.destroy(x)` (if `x` is a class or interface object) or
+        `typeid(*x).destroy(x)` (if `x` is pointer to a struct) to destroy the referred entity.


Here it should just be .object.destroy(*x), no? It's faster and better documented.

Are you sure about object.destroy being faster?

TypeInfo_Struct.destroy:

druntime/src/object.d

Lines 1192 to 1201 in 18be8bf

final override void destroy(void* p) const

{

if (xdtor)

{

if (m_flags & StructFlags.isDynamicType)

(*xdtorti)(p, this);

else

(*xdtor)(p);

}

}

object.destroy:

druntime/src/object.d

Lines 2954 to 2965 in 18be8bf

void destroy(T)(ref T obj) if (is(T == struct))

{

_destructRecurse(obj);

() @trusted {

auto buf = (cast(ubyte*) &obj)[0 .. T.sizeof];

auto init = cast(ubyte[])typeid(T).initializer();

if (init.ptr is null) // null ptr means initialize to 0s

buf[] = 0;

else

buf[] = init[];

} ();

}

The former is an indirect call and the second is a direct call. Truth be told, destroy does more work because it reinitializes the object. In the case of __delete we don't care to reinitialize the object because we just wrote in the documentation that access after __delete is undefined behavior.

So all in all the best way to go is to write in the documentation that we call destroy and actually call _destructRecurse. That covers all defined cases and is fast.

_destructRecurse is private and in object - so we can't even do package(core).

andralex · 2018-01-14T23:51:20Z

src/core/memory.d

+    $(LI
+        Calls `.object.destroy(x)` (if `x` is a class or interface object) or
+        `typeid(*x).destroy(x)` (if `x` is pointer to a struct) to destroy the referred entity.
+        Arrays of structs call `typeid(*x).destroy(x)` for each element.


I think it should be .object.destroy(e) for each element e in the array.

andralex · 2018-01-14T23:52:49Z

src/core/memory.d

+    // See also: https://github.com/dlang/dmd/blob/v2.078.0/src/dmd/e2ir.d#L3886
+    static if (is(T == interface))
+    {
+        (cast(Object) x).destroy;


I think this should do what the spec says it does: .object.destroy(cast(Object) x); This is a pedantic point because Object does not define a method destroy.

andralex · 2018-01-14T23:54:08Z

src/core/memory.d

+    }
+    else static if (is(T == class))
+    {
+        (cast(Object) x).destroy;


Same here, .object.destroy(cast(Object) x); would clarify lookup

andralex · 2018-01-14T23:57:48Z

src/core/memory.d

+    else static if (is(T == U*, U))
+    {
+        static if (is(U == struct))
+            typeid(U).destroy(x);


As mentioned, I think this is less efficient - it entails an indirect call instead of simply calling the destructor (if defined) or doing nothing (if not). I suggest: .object.destroy(*x)

andralex · 2018-01-14T23:57:59Z

src/core/memory.d

+        static if (is(E == struct))
+        {
+            foreach (ref e; x)
+                typeid(E).destroy(&e);


andralex · 2018-01-14T23:58:59Z

src/core/memory.d

+
+    static if (is(T == interface) || is(T == class) || is(T == U2*, U2) || is(T : E2[], E2))
+    {
+        GC.free(&x);


I'm not sure about this in the case of classes and interfaces. For those shouldn't the call be GC.free(cast(void*) x)?

Added tests for GC.free

wilzbach · 2018-01-15T03:15:58Z

src/core/memory.d

+$(UL
+    $(LI
+        Calls `.object.destroy(x)` (if `x` is a class or interface object) or
+        `(*x).__xdtor()` (if `x` is pointer to a struct and a custom destructor exists) to destroy the referred entity.


That's exactly what we do, but I wasn't whether there's a more elegant way to put it - __xdtor is a bit ugly, but it's still better than ".object.destroy(*x), but doesn't initialize the struct to T.init"

People cannot be expected to know what __xdtor does, but they do know what a destructor is. So the text should be:

"Calls the destructor ~this() for the object referred to by x (if x is a class or interface reference) or for the object pointed to by x (if x is a pointer to a struct). If no destructor is defined, this step has no effect."

wilzbach · 2018-01-25T00:01:55Z

So are we ready to 🚀 here?

andralex

A few more nits to look at.

andralex · 2018-02-01T23:39:16Z

src/core/memory.d

    pragma(mangle, "free") void fakePureFree(void* ptr);
 }
+
+extern(C) private @system nothrow


they are @nogc as well

andralex · 2018-02-01T23:40:54Z

src/core/memory.d

+The `delete` keyword allowed to free GC-allocated memory.
+As this is inherently not `@safe`, it has been deprecated.
+This function has been added to provide an easy transition from `delete`.
+It performs the same functionality as `delete`.


Instead of assuming people know delete and defining __delete in terms of it, describe __delete as "destroy then deallocate" and at the very end "see also the deprecated keyword" etc.

andralex · 2018-02-01T23:45:54Z

src/core/memory.d

+$(UL
+    $(LI
+        Calls `.object.destroy(x)` (if `x` is a class or interface object) or
+        `(*x).__xdtor()` (if `x` is pointer to a struct and a custom destructor exists) to destroy the referred entity.


People cannot be expected to know what __xdtor does, but they do know what a destructor is. So the text should be:

"Calls the destructor ~this() for the object referred to by x (if x is a class or interface reference) or for the object pointed to by x (if x is a pointer to a struct). If no destructor is defined, this step has no effect."

andralex · 2018-02-01T23:51:03Z

src/core/memory.d

+        Calls `.object.destroy(x)` (if `x` is a class or interface object) or
+        `(*x).__xdtor()` (if `x` is pointer to a struct and a custom destructor exists) to destroy the referred entity.
+        Arrays of structs with a custom destructor call `x.__xdtor()` for each element in the array.
+    )


"Arrays of structs call the destructor, if defined, for each element in the array."

andralex · 2018-02-01T23:51:53Z

src/core/memory.d

+void __delete(T)(ref T x) @system
+{
+    static void _destructRecurse(S)(ref S s)
+        if (is(S == struct))


andralex · 2018-02-01T23:53:39Z

src/core/memory.d

+    {
+        static if (__traits(hasMember, S, "__xdtor") &&
+                   // Bugzilla 14746: Check that it's the exact member of S.
+                   __traits(isSame, S, __traits(parent, s.__xdtor)))


I see the bugzilla is fixed, why do we need this?

Because fixing 14746, added this to _destructRecurse: 1a2290b

andralex

ping @wilzbach

wilzbach · 2018-02-06T23:15:17Z

ping @wilzbach

Sorry. Pong. Addressed the final nits & squashed everything into one commit.

wilzbach requested a review from andralex as a code owner January 11, 2018 18:25

wilzbach mentioned this pull request Jan 11, 2018

Fix Issue 9433 - Deprecate delete dlang/dmd#7342

Merged

wilzbach added the Blocking Other Work label Jan 11, 2018

JinShil suggested changes Jan 11, 2018

View reviewed changes

wilzbach force-pushed the __delete branch from b47b18f to 1b7292f Compare January 12, 2018 11:24

JinShil suggested changes Jan 12, 2018

View reviewed changes

andralex suggested changes Jan 13, 2018

View reviewed changes

wilzbach force-pushed the __delete branch 4 times, most recently from 915aeab to 444ab75 Compare January 13, 2018 09:37

JinShil suggested changes Jan 13, 2018

View reviewed changes

andralex reviewed Jan 13, 2018

View reviewed changes

wilzbach force-pushed the __delete branch from 93e2c1a to e726c10 Compare January 13, 2018 16:28

wilzbach commented Jan 13, 2018

View reviewed changes

wilzbach commented Jan 14, 2018

View reviewed changes

andralex suggested changes Jan 14, 2018

View reviewed changes

wilzbach force-pushed the __delete branch from eedd39f to b621ffb Compare January 15, 2018 03:12

wilzbach commented Jan 15, 2018

View reviewed changes

wilzbach force-pushed the __delete branch from ae90775 to 8ebf7cb Compare January 26, 2018 22:15

andralex approved these changes Feb 1, 2018

View reviewed changes

andralex reviewed Feb 5, 2018

View reviewed changes

wilzbach force-pushed the __delete branch 2 times, most recently from a675d5c to 57cbf25 Compare February 6, 2018 23:13

Add __delete for migration from delete

98e51b1

wilzbach force-pushed the __delete branch from 57cbf25 to 98e51b1 Compare February 6, 2018 23:14

andralex added the auto-merge label Feb 7, 2018

dlang-bot merged commit 117c237 into dlang:master Feb 7, 2018

JinShil mentioned this pull request Feb 8, 2018

Fix Issue 9433 - Deprecate delete dlang/dlang.org#1941

Merged

wilzbach deleted the __delete branch February 8, 2018 04:03

This was referenced Feb 23, 2018

[Trivial] Fix deprecated use of delete(x) in favor of x.destroy() #2104

Closed

Fix Issue 18505 - delete deprecation message is misleading dlang/dmd#7947

Merged

	void destroy(T)(T obj) if (is(T == class))
	{
	rt_finalize(cast(void*)obj);
	}

	/// ditto
	void destroy(T)(T obj) if (is(T == interface))
	{
	destroy(cast(Object)obj);
	}

	debug(PRINTF) printf("rt_finalize2(p = %p)\n", p);

	auto ppv = cast(void**) p;
	if(!p \|\| !*ppv)
	return;

	auto pc = cast(ClassInfo) ppv;
	try
	{
	if (det \|\| collectHandler is null \|\| collectHandler(cast(Object) p))
	{
	auto c = *pc;
	do
	{
	if (c.destructor)
	(cast(fp_t) c.destructor)(cast(Object) p); // call destructor
	}
	while ((c = c.base) !is null);
	}

	if (ppv[1]) // if monitor is not null
	_d_monitordelete(cast(Object) p, det);

	if (resetMemory)
	{
	auto w = (*pc).initializer;
	p[0 .. w.length] = w[];
	}
	}
	catch (Exception e)
	{
	import core.exception : onFinalizeError;
	onFinalizeError(*pc, e);
	}
	finally
	{
	*ppv = null; // zero vptr even if `resetMemory` is false
	}

	extern (C) void _d_delstruct(void** p, TypeInfo_Struct inf)
	{
	if (*p)
	{
	debug(PRINTF) printf("_d_delstruct(%p, %p)\n", p, cast(void)inf);

	inf.destroy(*p);

	if (xdtor)
	{
	if (m_flags & StructFlags.isDynamicType)
	(*xdtorti)(p, this);
	else
	(*xdtor)(p);

	void finalize_array(void* p, size_t size, const TypeInfo_Struct si)
	{
	// Due to the fact that the delete operator calls destructors
	// for arrays from the last element to the first, we maintain
	// compatibility here by doing the same.
	auto tsize = si.tsize;
	for (auto curP = p + size - tsize; curP >= p; curP -= tsize)
	{
	// call destructor
	si.destroy(curP);

	final override void destroy(void* p) const
	{
	if (xdtor)
	{
	if (m_flags & StructFlags.isDynamicType)
	(*xdtorti)(p, this);
	else
	(*xdtor)(p);
	}
	}

	void destroy(T)(ref T obj) if (is(T == struct))
	{
	_destructRecurse(obj);
	() @trusted {
	auto buf = (cast(ubyte*) &obj)[0 .. T.sizeof];
	auto init = cast(ubyte[])typeid(T).initializer();
	if (init.ptr is null) // null ptr means initialize to 0s
	buf[] = 0;
	else
	buf[] = init[];
	} ();
	}

Uh oh!

Add __delete for migration from delete #2037

Add __delete for migration from delete #2037

Uh oh!

Conversation

wilzbach commented Jan 11, 2018

Uh oh!

dlang-bot commented Jan 11, 2018

Bugzilla references

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ibuclaw commented Jan 12, 2018

Uh oh!

ibuclaw commented Jan 12, 2018

Uh oh!

JinShil commented Jan 12, 2018

Uh oh!

ibuclaw commented Jan 12, 2018

Uh oh!

wilzbach commented Jan 12, 2018

Uh oh!

JinShil Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JinShil Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ibuclaw commented Jan 12, 2018

Uh oh!

jmdavis commented Jan 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andralex commented Jan 13, 2018

Uh oh!

wilzbach commented Jan 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andralex commented Jan 13, 2018

Uh oh!

Choose a reason for hiding this comment

Exact behavior

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

schveiguy commented Jan 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JinShil Jan 12, 2018 •

edited

Loading

JinShil Jan 12, 2018 •

edited

Loading

schveiguy commented Jan 13, 2018 •

edited

Loading

andralex commented Jan 13, 2018 •

edited

Loading