Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add destruct function #929

Closed
wants to merge 1 commit into from
Closed

Conversation

denis-sh
Copy link
Contributor

@denis-sh denis-sh commented Nov 6, 2012

[EDITED]

We definitely need a function for object destruction which do exactly the same a compiler do in a case an object goes out of scope. First of all, it is needed for template programming:

struct S { T t; }

T* allocT() { ... }
...
T* p = allocT();
...
destruct(*t);

// These two calls must do the same as the first one do:
destruct(*cast(T[1]*) t);
destruct(*cast(S*) t);

An example of usage is an analog of a struct with manual memory management. Let us want to destruct such struct-analog:

void destructIt()
{
    ...
    foreach(i, T; Types)
        destruct(* cast(T*) getMemory(i)); // must destruct the object, not the reference
    ...
}

Also note, that druntime isn't a place for template code at all so proposed destruct function should definitely be in phobos.

Not sure about module where to place destruct.

Requires pull #928, assumes pull dlang/druntime#344.

Note, that it destructs a given object, but not something it references.
So to destruct a class instance, use $(D object.finalizeClassInstance).
*/
void destruct(T)(ref T t, bool resetInitialState = true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this auto ref and make it so no assignment is done if the parameter isn't assignable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, you are proposing to support this:

import std.typecons: destruct;

struct S { ... }

S f() { ... }

void main()
{
    destruct(f());
}

Practical use cases?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Practical use cases?

Why on earth should you have to be destroying a variable? destroy certainly doesn't require it. Being able to set a reference or pointer to null after destroying what it points to makes sense, but why restrict it to only work with lvalues? Limiting it to ref is artificially restrictive, since the fact that you're destroying something doesn't have anything to do with whether it's an lvalue or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why on earth should you have to be destroying a variable? destroy certainly doesn't require it. Being able to set a reference or pointer to null after destroying what it points to makes sense, but why restrict it to only work with lvalues? Limiting it to ref is artificially restrictive, since the fact that you're destroying something doesn't have anything to do with whether it's an lvalue or not.

When we are destructing something by hands, it means that we have a reference to it which scope lasts longer than we need. And if we have no variable, we have no scope problems: just write { f(); } instead of destruct(f());.

Possibly I'm loosing some useful cases, so I'm asking again for practical use cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All it takes is having what you're looking to destroy be returned from a function (e.g. the object that had ownership of it is handing it off to you). If you force ref, then that would have to be assigned to a variable instead of just fed directly to destroy/destruct. Regardless, requiring ref restricts it to lvalues when there's no reason not to work with rvalues`.

And I still want to know why this function is being created at all. This is what destroy is for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, why haven't you already discussed you no-ref proposal for current object.destroy?
Second, I don't understand it at all:

All it takes is having what you're looking to destroy be returned from a function (e.g. the object that had ownership of it is handing it off to you). If you force ref, then that would have to be assigned to a variable instead of just fed directly to destroy/destruct.

Why not just give an example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just give an example?

How is that not an example. If you have an object which owns an object, and it has a function which relinquishes ownership of that object to the caller and returns it, then if destruct takes ref, then you'd have to assign it to a variable first just to destroy it. e.g.

auto obj = owner.relinquishOwnership(key);
destruct(obj);

instead of

destruct(owner.relinquishOwnership(key));

Regardless, unless specifically requiring an lvalue buys you something (and I don't see why it would), then I see no reason to restrict the function to lvalues. Just use auto ref.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I already wrote it ( just write { f(); } instead of destruct(f()); ) I still do not understand what do you want.
Probably, you misunderstand the situation.

If you need a statement, you do this:

// statements
destruct(owner.relinquishOwnership(key)); // your variant

{ owner.relinquishOwnership(key); } // variant without `destruct`
// statements

If you need an expression, you do this:

// your variant
(/* expressions */ , destruct(owner.relinquishOwnership(key)), /* expressions */ ) ;

// variant without `destruct`
(/* expressions */, { owner.relinquishOwnership(key); }(), /* expressions */ ) ;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexrp, can you post you opinion here too, please?

@jmdavis
Copy link
Member

jmdavis commented Nov 6, 2012

Why do we need this? Isn't this what destroy is for?

@denis-sh
Copy link
Contributor Author

denis-sh commented Nov 7, 2012

Why do we need this? Isn't this what destroy is for?

I wrote my reasons to supersede destroy in dlang/druntime#344 description.
Why are you silently assume that all my reasons are incorrect and don't give any arguments?

@ghost
Copy link

ghost commented Nov 7, 2012

You're referencing the wrong pull (phobos pull 344 instead of druntime), it should be this: dlang/druntime#344

@denis-sh denis-sh mentioned this pull request Nov 9, 2012
@ghost ghost mentioned this pull request Nov 16, 2012
@dnadlinger
Copy link
Member

@denis-sh: I doubt your assumption that »druntime isn't a place for template code at all« – how did you come to this conclusion?

@denis-sh
Copy link
Contributor Author

Almost every template code requires std.traits which is the base for template programming. As it is std.traits, not core.traits, there is no non-error-prone way to use templates in druntime.

@andralex
Copy link
Member

andralex commented Dec 9, 2012

I think this function is unnecessary and dangerous. Will close this now.

@andralex andralex closed this Dec 9, 2012
@denis-sh
Copy link
Contributor Author

denis-sh commented Dec 9, 2012

I think this function is unnecessary and dangerous. Will close this now.

Really? How ironic... This pull contains the only function usable for object destruction and it is rejected.
Damn. Probably I have to stop dealing with Phobos as this goes beyond all bounds...

@ghost
Copy link

ghost commented Dec 9, 2012

@klickverbot AFAIK presence of templates inside a library prevents from making it a dynamic - that's why D, unlike C statically links to its runtime library and that's why phobos static library includes druntime which makes D binaries so bloated.

@jmdavis
Copy link
Member

jmdavis commented Dec 9, 2012

Templates do not prevent a library from being dynamic. It's just that the templates themselves aren't really part of the dynamic library, because they're instantiated when used, and there's no way to include every possible instantiation in the dynamic library. C++ is in exactly the same boat. Templates must go in header files, and it's mostly what's in the source files which end up in the library, whereas the header files get recompiled every time that they're included.

The fact that D statically links to its runtime stems from the fact that the runtime doesn't support dynamic libraries yet. Once it does, it will likely be dynamically linked rather than statically linked. Templates have nothing to do with it.

@monarchdodra
Copy link
Collaborator

According to this thread:
http://forum.dlang.org/thread/bdzwpmehiazpicftbxko@forum.dlang.org

The bug is in phobos:
https://github.com/D-Programming-Language/phobos/blob/83d603d00b3ee0d0cec5c7498eb478bd30c9f6c7/std/container.d#L2479

Apparently, when being destroyed, Array simply calls ".destroy" on all its items. Including if the items are class types. This results in premature destruction, general program corruption.

@denis-sh
Copy link
Contributor Author

@monarchdodra, could I ask you for help in inclusion in Phobos a proper implementation of functions for user-defined lifetime from unstd.lifetime? It is not generally useful stuff for an averege user so I don't expect any feedback at NG but it's critical for e.g. collections and typed manual memory allocators. So I'll be happy to be able to merge this facility.

Anyway thanks for comment. In ancient times I tried to add stuff to Phobos "I think this function is unnecessary and dangerous" was a common answer and all my arguments was considered insufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants