Hi Dimitri,

On Oct 8, 2009, at 11:54 PM, Dimitri Tcaciuc wrote:

The attached is the proposed fix and test case for 5077.

Thank you! Some comments below.

I've discussed it for some time on IRC and it seems this is not a
perfect solution, however at this point I'm a bit stumped to find an
alternative and I figured I would get more results with mail list
discussions. The train of thought is the following:

* Member initializers can accept one or more expressions which can
emit temporaries that need to be cleaned up.

Right.

  * Assuming expressions are correctly generated and use
EmitCXXExprWithTemporaries, can we assume that there can only be one
leftover temporary per argument?

I don't think we can assume that. Any given argument could involve computing many temporaries. For example, our initializer could be

        std::string() + "hello" + ", " + world

which has 4 temporaries.

* The temporary needs to persist until after member constructor call.

Specifically, until the end of the "full expression", which is the member initializer itself. It doesn't have to be a constructor call to have temporaries.

* CXXBaseOrMemberInitializer is not an Expr and cannot be wrapped
with EmitCXXExprWithTemporaries

Right.

* (?) There should be no meaningful temporaries left after member
initializer completes.

Temporaries can live after the member initializer completes if they were bound to a reference member (12.2p5). In this case, the temporaries are destroyed at the end of the constructor.

Thus, we can do the same thing as in EmitCXXExprWithTemporaries , that
is perform a check right after member constructor completes and any
new live temporaries still kicking around are safe to dispose of.

Started nibbling on clang only a relatively short time ago, so
comments and guidance are much appreciated!

I think your approach is right. However, I think that the cleanup code

+          // Cleaning up whatever temporaries occurred due to
+          // argument expressions to member intializer.
+          while (LiveTemporaries.size() > OldNumLiveTemporaries) {
+            PopCXXTemporary();
+          }

is in the wrong place, since temporaries can be created for base-class initializers and member initializers that don't involve constructor calls, in addition to the case you're currently handling. I suggest putting this temporary-cleanup code at the end of the "for" loop (which you'll need to restructure a little bit due to the "continue"), so that we handle temporaries the same way for the various different kinds of ctor-initializers.

I've attached an executable test case that shows some temporaries being created when the members are not class types. I believe that the current output is:

Before construction
Default-constructed X<1>
Destroyed X<1>
Default-constructed X<2>
Default-constructed X<3>
Destroyed X<3>
Default-constructed X<4>
In constructor body
Destroyed X<4>
Before destruction
Destroyed X<2>

Although GCC 4.2 seems to be destroying the temporary bound to HasMembers::i4 too early (it happens before "In constructor body").

Thanks for working on this!

        - Doug

Attachment: meminit.cpp
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to