http://d.puremagic.com/issues/show_bug.cgi?id=9433

           Summary: Deprecate delete
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Keywords: accepts-invalid
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: bearophile_hugs@eml.cc


--- Comment #0 from bearophile_hugs@eml.cc 2013-01-31 03:59:34 PST ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433


FG <home@fgda.pl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |home@fgda.pl


--- Comment #1 from FG <home@fgda.pl> 2013-01-31 07:14:06 PST ---
(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?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433


rswhite4@googlemail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rswhite4@googlemail.com


--- Comment #2 from rswhite4@googlemail.com 2013-01-31 07:37:20 PST ---
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".

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg@gmx.com


--- Comment #3 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-01-31 08:12:00 PST ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433



--- Comment #4 from rswhite4@googlemail.com 2013-01-31 08:29:13 PST ---
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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433


Iain Buclaw <ibuclaw@ubuntu.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ibuclaw@ubuntu.com


--- Comment #5 from Iain Buclaw <ibuclaw@ubuntu.com> 2013-01-31 08:37:17 PST ---
(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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433



--- Comment #6 from rswhite4@googlemail.com 2013-01-31 08:44:46 PST ---
(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).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433



--- Comment #7 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-01-31 08:45:41 PST ---
> 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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433



--- Comment #8 from FG <home@fgda.pl> 2013-01-31 08:49:18 PST ---
(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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=9433



--- Comment #9 from rswhite4@googlemail.com 2013-01-31 08:57:26 PST ---
> 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. :)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------