Skip to content

fix issue 10055 - Incorrect attribute merging in destructor building #2003

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

Closed
wants to merge 1 commit into from

Conversation

hpohl
Copy link
Contributor

@hpohl hpohl commented May 10, 2013

@9rnsr
Copy link
Contributor

9rnsr commented May 10, 2013

Wait, why the three fail_compilation tests fails? For example:

struct S { ~this() { } }
struct SX { S s; @safe ~this() { } }

Even if user defined dtor in SX is @safe, whole destructor of SX should be deduced to @System and compilation should be succeeded.

I think that is necessary behavior, because we cannot make destructors template. In template struct, destructor attributes of the fields are sometimes undetermined, so current behavior would make some template struct unusable...

@9rnsr
Copy link
Contributor

9rnsr commented May 10, 2013

I think this pull request is wrong.

@hpohl
Copy link
Contributor Author

hpohl commented May 10, 2013

I think that a dtor qualified with pure means that the destruction of this object is pure. It is forced to be pure. If any member prevents the object to be destructed purely, throw an error. So the destruction of the members takes place in the dtor (which the programmer can see, if declared), not in the "whole dtor".

In the case of template structs, you can resolve that issue using conditional compilation etc.

@9rnsr
Copy link
Contributor

9rnsr commented May 10, 2013

In the case of template structs, you can resolve that issue using conditional compilation etc.

The current behavior is very unuseful for defining destruction operation "much as possible pure/nothrow/@safe". Destrucuctors cannot be template, so you cannot rely on the compiler attribute deduction feature. Manually implementing it would require huge complex meta-programming. It is unrealistic.

Moreover, this behavior change will break existing code with 2.063 release. I think it is already a regression, and should be fixed.

@hpohl
Copy link
Contributor Author

hpohl commented May 10, 2013

This being said, I really like to deduce as much as possible. But you have to do it in the right way. What about deducing every part of the destruction as a whole and prohibit specifying any dtor attributes.

@hpohl
Copy link
Contributor Author

hpohl commented May 10, 2013

I agree on the conditional compilation thing, but I guess we need some more input.

@9rnsr
Copy link
Contributor

9rnsr commented May 10, 2013

I opened #2006 to fix issue 10055 and related issues which I've explained above.

@9rnsr
Copy link
Contributor

9rnsr commented May 10, 2013

I guess we need some more input.

At least, the changed behavior will break existing code - it is definitely bad.

@WalterBright
Copy link
Member

Was this superceded by #2006 ?

@hpohl
Copy link
Contributor Author

hpohl commented May 10, 2013

This pull request actually causes a different behavior: http://forum.dlang.org/thread/cfdbahwezrcdripbvhpz@forum.dlang.org

Maybe you can state which one is intended and close this depending on your decision.

@9rnsr 9rnsr closed this May 11, 2013
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.

3 participants