Sign in to follow this  
Followers 0
jdub

passing std::shared_ptr to a function?

25 posts in this topic

I have this code which causes a strange error:

void NBodySim::DrawCSOutput(std::shared_ptr<NBody::Renderer> graphics)
	{
		try
		{
			graphics->SBatch->Begin();

			RECT destRect;
			destRect.left = 0;
			destRect.right = 256;
			destRect.bottom = 256;
			destRect.top = 0;

			graphics->SBatch->Draw(this->uavSrv, destRect);

			graphics->SBatch->End();
		}
		catch(std::exception e)
		{
			const char *wat = e.what();
			int i = 0;
		}
	}

//DrawCSOutput() is called like this:
this->sim.DrawCSOutput(std::shared_ptr<NBody::Renderer>(this->graphics));

//this is the graphics member variable:
Renderer graphics;

I have managed to fix the code by changing the function to take a weak pointer instead but I am wondering, how can I successfully pass std::shared_ptr<T> as an argument to a function?

0

Share this post


Link to post
Share on other sites

The problem with the code above is that after the function call, the shared pointer will end up with 0 references and destroy the object (as it is the ONLY shared pointer to own that object).

 

 

If you would like to pass in a shared_ptr but keep the object alive, store your member as a shared_ptr and then pass it in to the function like so:

void NBodySim::DrawCSOutput(std::shared_ptr<NBody::Renderer> graphics)
	{
		try
		{
			graphics->SBatch->Begin();

			RECT destRect;
			destRect.left = 0;
			destRect.right = 256;
			destRect.bottom = 256;
			destRect.top = 0;

			graphics->SBatch->Draw(this->uavSrv, destRect);

			graphics->SBatch->End();
		}
		catch(std::exception e)
		{
			const char *wat = e.what();
			int i = 0;
		}
	}

//DrawCSOutput() is called like this:
this->sim.DrawCSOutput(this->graphics);

//this is the graphics member variable:
std::shared_ptr<Renderer> graphics;
3

Share this post


Link to post
Share on other sites

        catch(std::exception e)
        {
            const char *wat = e.what();
            int i = 0;
        }

 

'wat' indeed.

2

Share this post


Link to post
Share on other sites

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

2

Share this post


Link to post
Share on other sites

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.
1

Share this post


Link to post
Share on other sites

 

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

 

 

True, but there are also a few minor side effects.

 

First is that if the class overrides operator-> it will use that instead. This is a fairly rare thing, but something to consider.

 

Second is that it has the potential to cause some unnecessary trips to memory. Most optimizers will strip that away, but as this is the "General Programming" forum rather than "For Beginners", we should not be over-reliant on the optimizer. I've worked on several systems over the years (Nintendo DS, PalmOS, etc.) that when encountered would dereference the this pointer and follow it rather than use the information already loaded in registers.

 

 

My current studio includes it is in our programming style guide as a restricted practice. I flag it in code reviews as an error when I see it.

4

Share this post


Link to post
Share on other sites

 

 

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

 

 

True, but there are also a few minor side effects.

 

First is that if the class overrides operator-> it will use that instead. This is a fairly rare thing, but something to consider.

 

 

If class X overloads operator->, that affects using the operator-> on a pointer to X?

2

Share this post


Link to post
Share on other sites

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

 
True, but there are also a few minor side effects.
 
First is that if the class overrides operator-> it will use that instead. This is a fairly rare thing, but something to consider.
 
Second is that it has the potential to cause some unnecessary trips to memory. Most optimizers will strip that away, but as this is the "General Programming" forum rather than "For Beginners", we should not be over-reliant on the optimizer. I've worked on several systems over the years (Nintendo DS, PalmOS, etc.) that when encountered would dereference the this pointer and follow it rather than use the information already loaded in registers.
 

My current studio includes it is in our programming style guide as a restricted practice. I flag it in code reviews as an error when I see it.

As pointed out above, overriding -> has no effect on this.

Unless working on a platform where you must use a poor compiler, performing optimizations that the compiler can do for you trivially is premature optimization and should not be a major consideration in program style. A more reasonable objection is that it's verbose, and I think that's the real trade off here. Also using this-> has the advantage that it makes it easy to identify which variables are members.
1

Share this post


Link to post
Share on other sites

 

 

 

What is this fad of people using 'this->member' all over the place instead of just referring to members the normal way? It makes my teeth itch.

It's easier to use intelisense with.

 

 
True, but there are also a few minor side effects.
 
First is that if the class overrides operator-> it will use that instead. This is a fairly rare thing, but something to consider.
 
Second is that it has the potential to cause some unnecessary trips to memory. Most optimizers will strip that away, but as this is the "General Programming" forum rather than "For Beginners", we should not be over-reliant on the optimizer. I've worked on several systems over the years (Nintendo DS, PalmOS, etc.) that when encountered would dereference the this pointer and follow it rather than use the information already loaded in registers.
 

My current studio includes it is in our programming style guide as a restricted practice. I flag it in code reviews as an error when I see it.

 

As pointed out above, overriding -> has no effect on this.

 

 

 
That is what makes it confusing, and one of the reasons to avoid it.
 
The arrow operator is not overriden very often, but it is one of those "gotcha" areas of C++.  It generally works the way programmers expect it to work, but there is some subtlety in the drill-down behavior when operator overloading is involved.
 
Contrived example to demonstrate:

#include <iostream>
 
struct Concrete {
    void foo() { std::cout << "Concrete::foo\n"; }
};
struct Proxy {
    Concrete *a;
    void foo() { std::cout << "Proxy::foo\n"; }
    Concrete* operator->() { return a; }
};
struct Proxy2 {
    Proxy *b;
    void foo() { std::cout << "Proxy2::foo\n"; }
    Proxy& operator->() { return *b; }
 
    void RunFoo() {
        foo(); // Calls Proxy2::foo
        this->foo();  // QUIZ #1:  Which foo() is called?
        (*this).operator->()->foo(); // QUIZ #2: Which foo() is called?
    }
};
int main()
{
    Concrete a;
    Proxy b = { &a };
    Proxy2 c = { &b };
    c.RunFoo();
    c->foo(); // QUIZ #3: Which foo() is called?
}

 
The answers are not surprising at all when you stop to think about them.
 
The problem is that you need to stop and think about them.

Edited by frob
edit: code box got messed up, fixing formatting.
1

Share this post


Link to post
Share on other sites
I don't see why you think that foo() is clearer than this->foo(). To me it's the opposite, because this-> restricts the possible places to look for foo().

There are unintuitive parts to the code above, particularly in how proxy2 acts as a pointer to Concrete, instead of a pointer to Proxy, but that is an unrelated problem.

I guess you could say that the distinction between (*this)->foo() and this->foo() may be easily overlooked, but (*this)->foo() is strange looking enough to warrant special attention. Similarly with (*this).operator->()->foo(), except I don't see why you would ever use that instead of (*this)->foo().
1

Share this post


Link to post
Share on other sites

I guess you could say that the distinction between (*this)->foo() and this->foo() may be easily overlooked, but (*this)->foo() is strange looking enough to warrant special attention. Similarly with (*this).operator->()->foo(), except I don't see why you would ever use that instead of (*this)->foo().

 

I don't know, maybe whenever you want to intentionally make something confusing to try to prove a point.

0

Share this post


Link to post
Share on other sites

And yet the fact that operator-> is automatically called recursively until it returns an actual pointer type is occasionally used to implement special behavior or convenient helper functions by using wrapper objects that implement it. It's often the easiest way to keep the calling code clean and blissfully ignorant of implementation details. Try implementing a special proxy on top of a basic proxy to some generic object without exploiting operator->.

 

Basically, the question is: what happened to old-fashioned ways to mark members like 'mMember' or 'member_' that don't have the potential for obscure side effects?

 

There is no ambiguity for functions either, as a function is either a member, global or called through some other object. Global functions should be in a namespace and "using namespace everything" is often preferring laziness over clear code. So suddenly foo() is obviously a member function, as otherwise it would be something like std::foo() or ::foo().

1

Share this post


Link to post
Share on other sites

And yet the fact that operator-> is automatically called recursively until it returns an actual pointer type is occasionally used to implement special behavior or convenient helper functions by using wrapper objects that implement it. It's often the easiest way to keep the calling code clean and blissfully ignorant of implementation details. Try implementing a special proxy on top of a basic proxy to some generic object without exploiting operator->.
 
Basically, the question is: what happened to old-fashioned ways to mark members like 'mMember' or 'member_' that don't have the potential for obscure side effects?
 
There is no ambiguity for functions either, as a function is either a member, global or called through some other object. Global functions should be in a namespace and "using namespace everything" is often preferring laziness over clear code. So suddenly foo() is obviously a member function, as otherwise it would be something like std::foo() or ::foo().

What's the side effect? this and *this are totally different objects, and anyone who confuses the two should not be using nested proxies.

I also disagree that global functions should always be namespace qualified, because doing so prevents argument dependent lookup. So for example, you should prefer to call unqualified swap, and a using statement rather than calling std::swap.
1

Share this post


Link to post
Share on other sites

The point is not that it causes ambiguity or strange behavior.

 

The point is that it makes the programmer stop for a moment to figure it out.  

 

Code should be obvious.  You should generally be able to tell at a glance what the code does.  Code that does something tricky should include a great big comment block explaining why it does what it does, the reason the programmer decided to take the less obvious route, and instructions for future developers who maintain it.

 

When I see "this->foo()" my brain needs to pause, if only for a moment, and ask why they are calling a function from the object rather than calling the function directly. 

 

Just the fact that my brain must switch gears for a moment, no matter how small that moment is, that is the issue.

3

Share this post


Link to post
Share on other sites
The only reason your brain needs to pause for a moment is because you're not used to the syntax.
1

Share this post


Link to post
Share on other sites

I already know that it's pointless and that it's annoying as hell. I'm just wondering where it came from.

 

Did people really start doing this just for intellisense? I mean JFC, just upgrade to 2012.

0

Share this post


Link to post
Share on other sites

I already know that it's pointless and that it's annoying as hell. I'm just wondering where it came from.
 
Did people really start doing this just for intellisense? I mean JFC, just upgrade to 2012.

It's the most compelling reason I have. If I type this-> I can then see all methods and members of a class. Type a few more letters, and see all of them with a certain prefix, like get or set. Saves on typing, checking names, and prevents typos.

This cannot work for unqualified names, in part because the set of candidates can be large, and in part because of argument dependent lookup. But when calling a method on a class, you don't want argument dependent lookup.
0

Share this post


Link to post
Share on other sites


The point is not that it causes ambiguity or strange behavior.

 

Your original point was that it causes side effects. Are side effects not ambiguity or strange behavior?

0

Share this post


Link to post
Share on other sites

 


This cannot work for unqualified names

 

 

That's odd, it does in my copy of VS2012...

 

 

And it still does in 2013 (it's actually working so well, that I feel extremely irritated when I have to go back to 2010 at work).

 

Their early versions had some serious issues, though. I ended up writing my code in Wordpad and pasting it to the IDE, because it insisted to turn "int i"  into "int IObscureWinAPIInterface" the moment you entered a space or ;

 

I'm also still trying to find a good example of hidden virtual functions, where the qualified name or "this->" are the only ways to make it compile (which also means that just using "this->" by default might hide a potential bug). Considering the actual code also involved overloads and templates, one could argue that it's a rare corner case anyway. I still prefer something less "noisy" like a prepended m or appended _.

0

Share this post


Link to post
Share on other sites

I remember reading somewhere that people started uselessly writing this->whatever() in C++ after it was required by some other language, like with other idioms where people dont adapt.

0

Share this post


Link to post
Share on other sites


I'm also still trying to find a good example of hidden virtual functions, where the qualified name or "this->" are the only ways to make it compile (which also means that just using "this->" by default might hide a potential bug).

Using this-> to qualify names is required for explicit disambiguation in a compliant C++ compiler when you have a member function in a templated class and a possible conflict could arise dues to the interaction of two-phase name lookup and the template arguments.  It's not going to occur with the Microsoft compiler because it is famously non-compliant when it comes to two-phase name lookup.  It's only really technically necessary when you're writing general-purpose template libraries where you have no control over the types used to instantiate the templates.

 

Other than that, the use of "this->" is just a wart, like a m_ prefix or a _ suffix.  It contributes a good deal to readability and I would encourage the use of some kind of member designating wart ("this->" is too long for my taste, but tastes vary), but I recognize most code is write-only disposable code and software maintenance is a dying art, like buggy-whip manufacturing.

0

Share this post


Link to post
Share on other sites

It's not going to occur with the Microsoft compiler because it is famously non-compliant when it comes to two-phase name lookup.

 
Thanks for that little detail. Explains why I wasted at least an hour trying to confuse the compiler and couldn't reproduce that particular issue. Maybe I should have just copied the example from Alexandrescu's book.

It contributes a good deal to readability and I would encourage the use of some kind of member designating wart ("this->" is too long for my taste, but tastes vary), but I recognize most code is write-only disposable code and software maintenance is a dying art, like buggy-whip manufacturing.


It is? Because for the last six years I spent at least 80% of my time staring at code and trying to fix bugs and add workarounds. With your own code you usually have a good idea where to look or change stuff, unfortunately due to the above, it's rare to get a chance to deal with your own code. Wait, I think my sarcasm radar might be broken...
0

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now
Sign in to follow this  
Followers 0