R: ownership of "this" pointer

[albert-mirzoyan]

Please consider this example

class Object;

void superDuperDelete(owner<Object*> p){
    //some work
    delete p;
    //other work
}

class Object {
    void deleteSuperPuper(){
        if(OK()){
            //either just delete 
            delete this;
        }
        else {
            //or pass ownership to someone, who will delete sometime
            superDuperDelete(this);
        }
    }
}

int main(int argc, char *argv[]){
    owner<Object*> op = new Object();
    Object* p = new Object();

    op->deleteSuperPuper(); //this is OK
    p->deleteSuperPuper(); //this is BAD

    return 0;
}

how do we handle this, how "this" is interpreted in these two cases? don't we need some way like "const" to define that "this" is owned by a function?

Something like this.

class Object {
    void deleteSuperPuper() owner {
    }
}

Good real-life example of this "pattern" is QObject::deleteLater() in Qt.

[ghost]

From my understanding the type of p should be owner<Object*>, following the Guidelines. There will be no issue then deleting, because the object to be deleted is actually owned.

Apart from that: delete this in the isocpp.org/wiki

[jwakely]

Even if the type of p is owner<Object*> there can still be other non-owning pointers, say p2, and so the problem remains. How do you mark a function to say it should only be called by owners?

The example of such a function given here destroys the object (either immediately or by deferring it) and I agree that's an important case. Are there other examples of "owner-only" functions? If not, then maybe the solution would be an attribute that says the function is morally equivalent to a destructor:

class Object {
  [[gsl::destructor]] void deleteSuperPuper();
};

This would allow checkers to enforce it is not called by non-owners, and would also allow them to enforce that the object is not accessed after such a call.

[albert-mirzoyan]

@jwakely thanks, exactly what I wanted to say, except the attribute name :) Actually the function may just transfer the ownership but do not destroy itself and naming it destructor is a bit aggressive.

[jwakely]

So "ownership sink" is the more general case, covering more than just destruction. Maybe [[gsl::disown]] ? (Or "abandon" or "renounce", but I like that "disown" has the word "own" in it.)

[albert-mirzoyan]

@jwakely What about compiler warnings on unsupported attribute? warning: 'gsl::disown' scoped attribute directive ignored [-Wattributes]

[gdr-at-ms]

If the compiler does not care about GSL for now, well you are in a tough spot. Talk to your compiler supplier.

[albert-mirzoyan]

The language provides no way of adding user defined attributes. For now at least. So this attribute is not going to be defined in GSL. Do you think adding a thing in guideline which will produce warnings is a good idea?

[jwakely]

Yes. It makes perfect sense to me for the guidelines to propose an attribute which will be recognised by any compilers and other tools which try to enforce the guidelines. That's the point of scoping attributes with a namespace like "gsl::", and there are ways to check for support:

https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations#recs.hasattr

#ifdef __has_cpp_attribute
#  if __has_cpp_attribute(gsl::disown)
#    define ATTR_DISOWN [[gsl::disown]]
#  endif
#endif
#ifndef ATTR_DISOWN
#  define ATTR_DISOWN
#endif

[jwakely]

Although such macros should be a stopgap for user-code and portability. Eventually I hope compilers would recognise gsl:: attributes and not warn, or provide a way to turn off warnings for specific attribute namespaces. The point is that the behaviour of today's compilers should not hold back the design of good guidelines.

[gdr-at-ms]

@albert-mirzoyan Yes, I think it is a good idea for the GSL to define its attributes in the gsl:: namespace. We encourage C++ compilers to recognize these attributes with intended semantics.
@jwakely is exactly right.

[albert-mirzoyan]

@gdr-at-ms I'm OK with that. Just wanted to point to the possible inconveniences.

[albert-mirzoyan]

Since we one a same page regarding the attribute. Is there any stopper expected in future for attribute name [[gsl::owner]]? At this point I don't see any problem to call it so, and think it is more consistent with both gsl::owner alias and const qualifier. So it would be more intuitive that the function which has "gsl::owner" is considering "this" to have a type gsl::owner<Object*>.

class Object {
  [[gsl::owner]] void deleteSuperPuper();
};

What do you think?

[neilmacintosh]

Just as a general comment: I would hope this pattern happens infrequently enough that it does not require explicit attribute-based support.

Having individual "destructor" functions that - unlike actual destructors - are not invoked in language-standard ways (such as at the end of a scope) is a fragile pattern that works against the aim of the approach of the lifetimes profile: to prevent leaks and dangling by leveraging existing language elements.

[albert-mirzoyan]

I realized this issue when I was working with Qt which has a QObject::deleteLeter() which simply defers the deletion of object by pushing it back to the event loop to make sure that there is no pending event in a loop which is going to use an object. whatever...

The main point of this proposal is to consider "this" pointer to be non-owning by default (otherwise any method is a backdoor for making mistakes), but have a way to make it owning if a function is going to move ownership inside out for any reason.

[neilmacintosh]

@albert-mirzoyan Thanks for clarifying the motivation, that helps. I've followed this thread, but I'm not sure I see any cases where the this pointer needs to be non-owning by default to avoid mistakes - except for the explicit delete this case.

[albert-mirzoyan]

Ok delete this is a clear case. But what about any function which has owner<Object*> in its argument. Shouldn't I expect that this function may delete an object or pass an ownership? I think I should. So calling any such function with "this" is a potential mistake since nobody knows what is the original object. This leads to "'this' pointer should be non-owning by default" since by default it is bad to move ownership of "this".

More general, how a method can be owning "this" if it has no idea where it came from.

[BjarneStroustrup]

this isn't an owner. Objects are destroyed "from the outside" by scope exit and delete. Some form of casting/annotation must be used to enable the relatively rare "delete this" (an inherently unsafe operation

• consider stack objects)

On 10/14/2016 8:48 PM, Albert wrote:

Ok |delete this| is a clear case. But what about any function which haa owner in its argument. Shouldn't I expect that this function may delete an object or pass an ownership? I think I should. So calling any such function with "this" is a potential mistake since nobody knows what is the original object. This leads to "'this' pointer should be non-owning by default" since by default it is bad to move ownership of "this".

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/isocpp/CppCoreGuidelines/issues/738#issuecomment-253951228, or mute the thread https://github.com/notifications/unsubscribe-auth/AKbTNExlwzbcYQTUqfdUhGkXBrIyidhtks5q0CLRgaJpZM4J7yCU.

[hsutter]

I think I can now see @albert-mirzoyan's point, because if Object::deleteme() were a nonmember function we would annotate the Object* parameter as an owner in some way so that it's clear the caller is relinquishing ownership. However, because Object::deleteme() is a member function, the this parameter is implicit and we would have to push the annotation elsewhere in the function declaration just as we do with cv-qualifiers and &-qualifiers for the this parameter.

However, because delete this is such a rare operation, and so rarely correct, I don't think this warrants an explicit language declaration extension like cv-qualifiers and &-qualifiers. So I'm quite happy with requiring that this be a non-owner always (not just by default) and so for the very rare cases like Object::deleteme() there are two options:

• Leave it as a nonstatic member function and do an explicit cast/annotation for delete this as @BjarneStroustrup suggested.
• Make it a static member function and then the parameter becomes visible and you can annotate its ownership directly as usual. You lose the ability to invoke it using member function call syntax, but that's arguably an advantage because it's such a special function that you probably want to call attention to it at call sites (and any kind of ownership annotation would force that anyway).

[albert-mirzoyan]

@hsutter Explicit cast will allow to write delete this in a method, but without function qualifiers static analyzer will not prevent the method user/caller from making a mistake.

With this example the analyzer would not say "hey you do not have an ownership to call this function"

int main(int argc, char *argv[]){
    Object* p = getObject();
    p->deleteLeter(); //this is BAD
    return 0;
}

I would like this error to be caught.

[gdr-at-ms]

@albert-mirzoyan Is your point that the member function would have been declared by mistake? Because, if it is part of the class's interface, calling it isn't a mistake. What would be a mistake is a poor class design; that isn't fixed by handing out an annotation.

[albert-mirzoyan]

@gdr-at-ms No, my point is about @hsutter's quote

I don't think this warrants an explicit language declaration extension like cv-qualifiers and &-qualifiers.

I think we should have this qualifier and an above example illustrates why.

[hsutter]

@albert-mirzoyan Please see my last reply -- why not make it a static function? Even without the GSL owner annotations, it would be better designed as a static function that takes, for example, an explicit unique_ptr<Object> parameter; the caller would be forced to explicitly relinquish ownership. And you can do that today.

More generally, I think the core issue here is that @albert-mirzoyan is proposing a new feature extension to the actual language to deal with a specific case, but this is not really the place for new language extension proposal discussion. These Guidelines and their minimal additional support library (GSL) are about how best to use the existing language, and occasionally how to prepare in anticipation of some already-in-progress near-future language features that are already coming through the committee.

For new standards proposals, see How to submit a standards proposal. While it does discourage language proposals, those who feel strongly about their language extension proposal can still go ahead and float those on the indicated std-proposals forum as well.

[albert-mirzoyan]

@hsutter I'm not proposing any feature extension to language. This discussion is all about guidelines and GSL.

I agree that the static function would be better design. Аlthough it is not covering the virtual function case ( please do not throw anything at me :) ).

On my first post I have asked two questions. And here are the answers (correct me if I'm wrong). "this" pointer should always be considered a non-owning pointer by static analyzer. There would be no qualifier like attribute to hint analyzer that the method is going to do some trick and own "this".

[BjarneStroustrup]

But something needs to be done to "rescue" all the old code relying on "delete this;" Right? My guess is that suppressing the warning for that at each occurrence in code is the simplest solution.

On 10/15/2016 6:04 PM, Albert wrote:

@hsutter https://github.com/hsutter I'm not proposing any feature extension to language. This discussion is all about guidelines and GSL.

I agree that the static function would be better design. Аlthough it is not covering the virtual function case ( please do not throw anything at me :) ).

On my first post I have asked two questions. And here are the answers (correct me if I'm wrong). "this" pointer should always be considered a non-owning pointer by static analyzer. There would be no qualifier like attribute to hint analyzer that the method is going to do some trick and own "this".

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/isocpp/CppCoreGuidelines/issues/738#issuecomment-254013830, or mute the thread https://github.com/notifications/unsubscribe-auth/AKbTNEi0dUOFrKgIu2Pf1cL9l1ecK2pSks5q0U3pgaJpZM4J7yCU.

[hsutter]

OK, I think we are all in agreement then: this is always nonowning with no way to annotate it as owning, and delete this; needs to explicitly suppress a warning.

[AndrewPardoe]

Herb will add a clarifying note in the Guidelines.