http://bugzilla.gdcproject.org/show_bug.cgi?id=70

--- Comment #1 from Marco.Leise@gmx.de 2013-07-08 10:02:32 UTC ---
See also: https://github.com/ldc-developers/ldc/issues/426

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

--- Comment #2 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-07-08 17:34:39 UTC ---
(In reply to comment #1)
> See also: https://github.com/ldc-developers/ldc/issues/426

No, I will not see also. :P

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

--- Comment #3 from Marco.Leise@gmx.de 2013-07-08 17:41:18 UTC ---
I cross-linked both reports, so if either of you compiler devs finds out what the issue is you can share it with the other team. That's all. So far on the LDC side it only reads "I can reproduce this". :)

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #4 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-07-08 22:52:40 UTC ---
https://github.com/D-Programming-GDC/GDC/commit/fa96279f9d932f3ebebd456d1f2a7a3db9626f9b

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

--- Comment #5 from Johannes Pfau <johannespfau@gmail.com> 2013-07-14 07:01:44 UTC ---
Created attachment 41
  --> http://bugzilla.gdcproject.org/attachment.cgi?id=41
Regression test case

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

Johannes Pfau <johannespfau@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |NEW
                 CC|                            |johannespfau@gmail.com
         Resolution|FIXED                       |

--- Comment #6 from Johannes Pfau <johannespfau@gmail.com> 2013-07-14 07:32:00 UTC ---
This bug fix has caused all remaining unit test failures (except the math ones). The problem is that a function can throw without actually having constructed the object and now we always call the dtor. A test case is attached.


What dmd does is pretty weird. It doesn't handle this in toElemDtor because of the reason stated above, there's absolutely no error handling in toElemDtor. It's also not handled in the frontend.

Once the complete IR of a function is available, (eh.c: except_fillInEHTable) iterates over all expressions. According to a comment dmd only handles temporaries where the ctor&dtor are called in the same expression this way.

So what we should do is after we have the GENERIC representation of a function, we have to do something like this pseudo code:

foreach(tree exp; functionIR)
{
    outer: foreach(i, tree exp2; compound(exp))
    {
        if(exp2.isctor)
        {
            size_t numCtor = 1;
            size_t indexFirstDtor = 0;
            foreach(j, tree exp3; compound(exp)[i+1 .. $])
            {
                if(exp.isctor)
                    numCtor++;
                else if(exp.isdtor)
                {
                    numCtor--;
                    if(indexFirstDtor == 0)
                        indexFirstDtor = j;
                    if(n == 0) //found all matching dtors
                    {
                        tree handler =
tryfinally(compound(exp)[i+1..firstDtor], compound(exp)[firstDtor..j+1]);
                        compound(exp)[i+1 .. j+1].replace(handler);
                        break outer; //AFAICS executing the inner loop once is
enough
                    }
                }
            }
        }
    }
}

Now I don't claim this code is pretty or correct, dmd has enough dtor related bugs and this may be the cause of some. But this is what the dmd code currently does.

In long term this needs to be fixed in the frontend though. I think we should
never rely on the glue layer to insert try/catch/finally statements. Instead
dmd should lower this "(DestroyMe __sl6 = DestroyMe(); ,
__sl6).opApply(delegate int(int item)" into "(DestroyMe __sl6 = DestroyMe();,
try{__sl6).opApply(delegate int(int item)}finally{__sl6.~this()})"


BTW: The unit test failure "null this" in parallelism.d is caused by the dtor _not_ being called as in the original test case. So if we revert this fix without applying a new fix, the std.parallelism test is the only failing test.

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

--- Comment #7 from Johannes Pfau <johannespfau@gmail.com> 2013-07-14 07:51:07 UTC ---
> tree handler = tryfinally(compound(exp)[i+1..firstDtor], compound(exp)[firstDtor..j+1]);

Sorry, the pseudo-code is partially wrong. To be 100% correct we have to find the matching dtor for all ctors, then add try/finally acordingly. E.g

a.ctor;
b.ctor;
doSomething();
b.dtor;
a.dtor;

should be
a.ctor;
try
{
  b.ctor;
  try
  {
     doSomething();
  }
  finally{b.dtor;}
}
finally {a.dtor;}

But I think dmd doesn't do this nesting which is probably the cause of http://d.puremagic.com/issues/show_bug.cgi?id=9704

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

--- Comment #8 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-07-14 10:57:48 UTC ---
(In reply to comment #5)
> Created attachment 41 [details]
> Regression test case

I think that regression would have been triggered against the patch above.  But about 15 minutes after closing this ticket, I realised this and rebased it to:

https://github.com/D-Programming-GDC/GDC/commit/9a301488360f692157d290ac3e5d3b1e41c5ee54

So I cannot reproduce that regression test.

Long-term fix would definitely be in the front-end though... I'm just putting in the quick fix now.

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

Johannes Pfau <johannespfau@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #9 from Johannes Pfau <johannespfau@gmail.com> 2013-07-15 14:37:33 UTC ---
I think you're right and I somehow managed to get the old version. I can't reproduce my test case with a new checkout and the original test case is also fixed.

So the only failing unittest left is "src/std/parallelism.d(3215): null this" ?

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.

http://bugzilla.gdcproject.org/show_bug.cgi?id=70

--- Comment #10 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-07-15 16:57:22 UTC ---
(In reply to comment #9)
> I think you're right and I somehow managed to get the old version. I can't reproduce my test case with a new checkout and the original test case is also fixed.
> 
> So the only failing unittest left is "src/std/parallelism.d(3215): null this" ?

Certainly looks like it:

http://d.puremagic.com/test-results/test_data.ghtml?projectid=2&runid=48270&logid=13

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.