Bug 9433 – Deprecate delete

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2013-01-31T03:59:34Z
Last change time
2018-02-23T14:35:27Z
Keywords
accepts-invalid
Assigned to
No Owner
Creator
bearophile_hugs
See also
https://issues.dlang.org/show_bug.cgi?id=11949

Comments

Comment #0 by bearophile_hugs — 2013-01-31T03:59:34Z
class Foo {} void main() { auto f = new Foo; delete f; } DMD 2.062alpha compiles that code with no warnings or errors. But I expect a message like: temp.d(5): Deprecation: use of delete is deprecated; use destroy (and core_memory.GC.free) instead.
Comment #1 by home — 2013-01-31T07:14:06Z
(In reply to comment #0) > But I expect a message like: > temp.d(5): Deprecation: use of delete is deprecated; use destroy (and > core_memory.GC.free) instead. Delete cannot be deprecated because there is no properly working replacement for delete at this moment. Look at the following code: import std.stdio, core.memory; class C { byte[] s; this() { s = new byte[40 * 1024 * 1024]; } } void main(string[] args) { for (int i=0, j=0; i < 1000; i++) { auto x = new C(); // delete x.s; // A // x.s.destroy(); // B // GC.free(cast(void*)x.s); // C } } Let's see memory usage on windows when line A, B or C get uncommented, with code compiled using DMD32 v2.060 and GDC (tmd64-1) 4.6.1: code | dmd32 | gdc64 - | OOM | 160MB A | 40MB | 40MB B | OOM | 160MB B+C | OOM | 160MB C | 40MB | 40MB where OOM = OutOfMemoryError and 160MB is sometimes 120MB. Only lines A and C are acceptable memory-wise but C doesn't run destructors. Combining C with B calls destructors but doesn't free memory. Even the manual for core.memory.free tells to use delete in such cases! So how can you get rid of delete?
Comment #2 by rswhite4 — 2013-01-31T07:37:20Z
Try this: [code] import std.stdio; void destruct(T)(ref T obj) if (is(T == class)) { .destroy(obj); core.memory.GC.free(cast(void*) &obj); // for valid state obj = null; } void destruct(T)(ref T chunk) pure nothrow if (!is(T == class)) { core.memory.GC.free(chunk); } void main() { for (int i = 0; i < 1000; i++) { ubyte[] chunk = new ubyte[40 * 1024 * 1024]; destruct(chunk); //delete chunk; } class A { ubyte[] _chunk; this() { this._chunk = new ubyte[40 * 1024 * 1024]; } ~this() { writeln("DTor"); destruct(this._chunk); } } for (int i = 0; i < 1000; i++) { writefln("start #%d run", i); A a = new A(); destruct(a); //delete chunk; writefln("end #%d run", i); } } [/code] I see no differences if I compare this and delete, except that "delete" looks nicer. But I have not made a detailed test, like you did. Maybe you could did the same with my "destruct".
Comment #3 by issues.dlang — 2013-01-31T08:12:00Z
delete has been on the chopping block for ages. I don't know why it hasn't actually been deprecated yet. It should have been deprecated quite some time ago. > Delete cannot be deprecated because there is no properly working replacement > for delete at this moment. There really isn't supposed to be a replacement for it. Part of the point is that what it's doing is fundamentally wrong. You shouldn't normally be freeing memory that's managed by the GC. That's the job of the GC, and it's unsafe for the programmer to do it. If you really want to be freeing memory yourself, then you should be managing it manually with malloc and free and not with new and delete. Now, manual memory management should become much nicer once custom allocators have been sorted out (allocating classes manually is a bit of a pain at the moment), but in general, if you're using delete, you're going about things the wrong way.
Comment #4 by rswhite4 — 2013-01-31T08:29:13Z
But you have to admit: if there is "new", there should be "delete" also. Because it is the natural opposite. Even if it is "unsafe", the possibility to free even GC memory by yourself, should be there. Otherwise we end up still as in the Java world, where you can not do anything by yourself. We should keep us more at C++ than at Java/C#. It is a great pain in Java, that you cannot free your memory by yourself. Sure the GC does that, but _when_ he does that is vague. I'm in favour, that delete should stay, without deprecating, but with a warning in the doc.
Comment #5 by ibuclaw — 2013-01-31T08:37:17Z
(In reply to comment #4) > But you have to admit: if there is "new", there should be "delete" also. > Because it is the natural opposite. That's a very black and white way of thinking. Also, just because Java does it badly doesn't mean that all languages that use a GC are deemed for the same failure too - Vala, C#, Haskell, Ruby, all successful languages in their own right that you wouldn't think to knock off because there is no delete keyword for every allocation that you call 'new' on (or even implicitly calls 'new' on). Iain.
Comment #6 by rswhite4 — 2013-01-31T08:44:46Z
(In reply to comment #5) > (In reply to comment #4) > > But you have to admit: if there is "new", there should be "delete" also. > > Because it is the natural opposite. > > That's a very black and white way of thinking. Also, just because Java does it > badly doesn't mean that all languages that use a GC are deemed for the same > failure too - Vala, C#, Haskell, Ruby, all successful languages in their own > right that you wouldn't think to knock off because there is no delete keyword > for every allocation that you call 'new' on (or even implicitly calls 'new' > on). > > Iain. That was not the main point of my post. And that should not have much weight. Whether it is "delete", "destroy" or "destruct" is does not matter. But we should keep the possibility, unsafe or not, to release manually GC Memory _and_ call the DTor (of course, only if it is an object).
Comment #7 by issues.dlang — 2013-01-31T08:45:41Z
> But you have to admit: if there is "new", there should be "delete" also. > Because it is the natural opposite. Except that new allocates GC memory, which is supposed to be managed by the GC. If you want to be managing memory yourself, you should use the tools intended for doing so - and that's not new or the GC. The language designers decided quite some time ago that delete was going to be deprecated. They just haven't gotten around to doing so. But it's definitely going to go. In fact, if they were to start over, they probably wouldn't have even had new (rather they would have had a library function which did the same thing), but they wouldn't have delete regardless. It was deemed too unsafe, and having it as a keyword makes it too obvious to use. At least with core.memory, it's much more obvious that what you're doing is dangerous and likely unwise. > Even if it is "unsafe", the possibility to free even GC memory by yourself, > should be there. That's what core.memory is for. It's there because D is a systems language, but it shouldn't be used normally. If you're using the GC for manual memory management (which is what you're doing if you're telling the GC to free memory), then you're using it incorrectly. Use malloc and free if you want to manage memory manually.
Comment #8 by home — 2013-01-31T08:49:18Z
(In reply to comment #2) > Try this: [...] > I see no differences if I compare this and delete, except that "delete" looks > nicer. But I have not made a detailed test, like you did. Nice, holds steady at 40MB, but there's a catch. When you remove the line "destruct(a);" and rely on the GC to remove unnecessary A instances, the program will crash, because GC.free cannot be called during a collection. So I think it's a bit too dangerous. OK, let's have a compromise. :) Delete isn't needed after all and can be removed. Since the problem is mostly about large arrays only, calling destructors is pointless, so using GC.free is fine to release those big chunks. The freeing could be hidden in A.clean() method: import std.stdio, core.memory; class A { ubyte[] _chunk; this() { this._chunk = new ubyte[40 * 1024 * 1024]; } void clean() { GC.free(this._chunk.ptr); this._chunk = null; } } void main() { for (int i = 0; i < 1000; i++) { A a = new A(); a.clean(); } } Is this approach OK? It keeps memory footprint 3-4 times smaller on 64-bit and prevents running out of memory on 32-bit windows. And I'll stop bugging you about keeping delete.
Comment #9 by rswhite4 — 2013-01-31T08:57:26Z
> Except that new allocates GC memory, which is supposed to be managed by the GC. > If you want to be managing memory yourself, you should use the tools intended > for doing so - and that's not new or the GC. The language designers decided > quite some time ago that delete was going to be deprecated. They just haven't > gotten around to doing so. But it's definitely going to go. In fact, if they > were to start over, they probably wouldn't have even had new (rather they would > have had a library function which did the same thing), but they wouldn't have > delete regardless. It was deemed too unsafe, and having it as a keyword makes > it too obvious to use. At least with core.memory, it's much more obvious that > what you're doing is dangerous and likely unwise. But GC.free does not calls any DTors. That is the problem. Therefore we need something like "destruct". @FG: > Nice, holds steady at 40MB, but there's a catch. When you remove the line > "destruct(a);" and rely on the GC to remove unnecessary A instances, the > program will crash, because GC.free cannot be called during a collection. So I > think it's a bit too dangerous. That's true, but it's the same if you use delete. It's an alternative. :)
Comment #10 by issues.dlang — 2013-01-31T09:03:21Z
> But GC.free does not calls any DTors. That is the problem. Therefore we need > something like "destruct". So, you call destroy before you call GC.free. It's not _supposed_ to be easy, because you're not supposed to be doing it. It's just supposed to be possible for the rare cases where it's actually needed.
Comment #11 by bearophile_hugs — 2013-01-31T10:10:24Z
(In reply to comment #7) > In fact, if they were to start over, they probably wouldn't have > even had new (rather they would have had a library function which > did the same thing), I leave the deprecation of "new" to another enhancement request ;-)
Comment #12 by issues.dlang — 2013-01-31T18:58:15Z
> I leave the deprecation of "new" to another enhancement request ;-) LOL. Yeah, well, it would break far too much code for that change at this point, so it's not going to happen.
Comment #13 by slavo5150 — 2017-11-21T05:20:54Z
Comment #14 by github-bugzilla — 2018-02-12T22:46:57Z
Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/f88ab395386b40f544c9f74ba768549c4fa9e66f Fix Issue 9433 - Deprecate delete https://github.com/dlang/dmd/commit/11daae8a0e859cf79de6ea94d6c1d6e7104381e9 Merge pull request #7342 from JinShil/deprecate_delete Fix Issue 9433 - Deprecate delete merged-on-behalf-of: unknown
Comment #15 by github-bugzilla — 2018-02-19T01:32:32Z
Commits pushed to master at https://github.com/dlang/dlang.org https://github.com/dlang/dlang.org/commit/3421e6a9904f11302719a50c033aceb68de0bf02 Fix Issue 9433 - Deprecate delete https://github.com/dlang/dlang.org/commit/363eb2dccf4aa19c4e8d9904feb31ac94a37df16 Merge pull request #1941 from JinShil/deprecate_delete Fix Issue 9433 - Deprecate delete
Comment #16 by schveiguy — 2018-02-23T14:35:27Z
*** Issue 11949 has been marked as a duplicate of this issue. ***