[PATCH] Have 'this'-returning constructors and destructors to take advantage of the new backend 'returned' attribute

Stephen Lin swlin at post.harvard.edu
Thu May 2 21:43:23 PDT 2013


> Ah, I'm forgetting which functions are used from where.
>
> I actually intentionally *don't* want this to be picked up in the same way
> when constructing a call.  For example, virtual calls to destructors
> cannot be marked with this attribute because none of these ABIs
> put this-adjustment thunks in the v-table.  So it's quite important that
> we only add this attribute on a call-site when directly calling a function.

Hmm, I *thought* the return type check adequately covers that already
actually: I have a test case to verify that non-virtual calls don't
return 'this':

+void test_destructor() {
+  E* e1 = new E();
+  E* e2 = gete();
+  delete e1;
+  delete e2;
 }
+
+
+// CHECKARM: define void @_Z15test_destructorv()
+
+// CHECKARM: {{%.*}} = call %class.E* @_ZN1EC1Ev(%class.E* %0)
+
+// CHECKARM: [[VFN:%.*]] = getelementptr inbounds void (%class.E*)** {{%v.*}}
+// CHECKARM: [[THUNK:%.*]] = load void (%class.E*)** [[VFN]]
+
+// Make sure only non-virtual calls to destructors return 'this'
+// CHECKARM: call void [[THUNK]](%class.E* {{%.*}})

but upon further reflection I'm not 100% sure this logic is sufficient
for any possible implementation of CGCXXABI with its current interface
or if any checks/asserts need to be added (even if they don't make a
difference currently). Let me examine it further...

However, it seems like basing it off HasThisReturn from the GlobalDecl
would be even worse, because the GlobalDecl will refer to the
destructor definition without any information about whether the call
is made virtually or not, as far as I can tell? I might be mistaken...

>
> That's also part of why I'm quite antsy about relying on the formal type
> of the ctor/dtor.  The guarantee isn't necessarily there just because the
> type is set up that way.
>
> I'm not sure there's a compelling reason for EmitCall to get a Decl*
> instead of a GlobalDecl.

Well, I don't mind refactoring this if it's possible, but is every
valid TargetDecl for EmitCall guaranteed to be a GlobalDecl, though?
Also, I'm not sure if actually helps in the current case, as above
(but if it might help for other reasons and is always valid, I can
work on it.)

-Stephen



More information about the cfe-commits mailing list