Thread: std::shared_ptr<T> implicit conversion from T*

Originally Posted by Elkvis

...so it may just be more advantageous for me to write my own much more basic shared pointer class for my purposes.

How about a convenience function rather than reinventing the wheel?

I guess I fail to see why implicitly constructing a shared_ptr from a raw pointer is any different from implicitly constructing a std::string from a const char pointer

That's actually a very different situation. The basic_string constructors, which are a part of the class that builds the whole string library (string, wstring, u16string, etc) have more than one argument. Some of the arguments are default, but that is still complex enough to rule out explicit construction. So the reason that you can implicitly construct strings from char pointers is because of the allocator argument forcing one particular resolution to the issue. One can say the risk is mitigated anyway, because of the type of string literals and how those values behave, and how they are stored and cannot actually be deleted. Char pointers can also be generated by containers such as vector<char>, where there is no particular risk of raw pointer shenanigans.

@whiteflags: There are two things at play (Example #2): constant "lvalue" temporaries and implicit versus explicit constructors. I feel that you are getting the two confused.

Thanks for your concern, but I didn't feel very confused. I just didn't want the discussion to become about lvalues and rvalues. It's more likely that Elkvis tried to get my example to work and failed in doing so, so he talked about it.

That's actually a very different situation.

O_o

Well, that is certainly true.

The basic_string constructors, which are a part of the class that builds the whole string library have more than one argument. Some of the arguments are default, but that is still complex enough to rule out explicit construction.

The different situation just has nothing to do with the constructor having more than one argument precisely because a form is available which allows implicit construction due to, as you say, default values. (Example #1)

One can say the risk is mitigated anyway, because of the type of string literals and how those values behave, and how they are stored and cannot actually be deleted.

The risk is in no way mitigated because of the type of string literals, how they are stored, nor whether they could be deleted.

There is no risk of the referenced data being deleted because `std::string' doesn't take ownership; that is the only reason the comparison to `std::string' is "safe". (Example #2)

Thanks for your concern, but I didn't feel very confused.

[Edit]
Ah, I wonder, by "confused" do you think I'm saying that you have "lvalue" issues confused with "implicit conversion" issues and also the reverse?

No, the problem you referenced in isolation is certainly a good reason to forbid implicit conversion for resource owning objects. I'm suggesting that you've bound the two issues at play into a single concern confusing the considerations of both as problems of either issue.
[/Edit]

I don't intend any offense, but I don't buy that you aren't still confusing the two issues.

The code you posted in post two does not match up, in any way, with your saying "Having the std::shared_ptr class's single argument constructor marked explicit helps prevent code like this from working.".

I included my example for you for exactly that reason. You didn't read it, presumably, because you "didn't feel very confused". Well, take a look at the example here using `std::string'. (Example #3) The code fails for the same reason as you reference in post four.

[Edit]

I just didn't want the discussion to become about lvalues and rvalues.

Well, that's the reason I think you are confusing the two issues.

You introduced "lvalue" related considerations into the thread.

I've added another code example which shows what you intended using your own labels. (Example #4) The example now shows what you intended without "lvalue" related problems. (I actually showed the same thing earlier, but you didn't bother with example apparently.)
[/Edit]

Soma

Code:

struct OTest1
{
    OTest1
    (
        int f1
      , int f2 = 0
    )
    {
    }
};

struct OTest2
{
    explicit OTest2
    (
        int f1
      , int f2 = 0
    )
    {
    }
};

void Go1(const OTest1 & f)
{
}

void Go2(const OTest2 & f)
{
}

int main()
{
    OTest1 s1 = 1;
    //OTest2 s2 = 1; // Fail!
    OTest2 s2(1);
    Go1(1);
    //Go2(1); // Fail!
    Go2(OTest2(1));
    return(0);
}

Code:

struct OTest
{
    OTest
    (
        const char * f
    )
    {
        delete f;
    }
};

void Go(const OTest & f)
{
}

int main()
{
    Go("This is BAD!");
}

Code:

#include <string>

void foobar(std::string & p){}

char * my_p = new char[16];

int main()
{
    foobar(my_p);
}

Code:

#include <memory>

void foobar(const std::shared_ptr<int> & p);
 
int main()
{
    int * my_p = new int;
    foobar(my_p);
}

I get that std::shared_ptr is intended to be extremely flexible and powerful, but it seems to me that there may be a legitimate need for a simple smart pointer that just manages memory for a normal object, and behaves like a normal pointer in every way that you'd normally use one, apart from array access.

Originally Posted by phantomotap

Hey, don't let me stop you, but let's for a second ignore the convenience thing which is the issue of this thread, have you ever had an issue using `std::smart_ptr' as "a simple smart pointer that just manages memory for a normal object"?

definitely not. it works very well for that, but as you pointed out, this thread is all about convenience.

Originally Posted by phantomotap

Still, don't me stop you; I don't think it take you more than a couple hours in any event.

for a smart pointer type that doesn't need to share control with other instances, exists only for the duration of the scope in which it was created, and doesn't do any "fancy stuff," a couple of hours is pretty generous.

I apologize.

O_o

Why?

The reason you gave was a good one; the only issue with the example was that the code was kind of colluding two different issues.

I only get frustrated when discussion goes nowhere; it sounds like to me like you are going to go refresh your knowledge of the issue.

This seems like something that was well worth the thread to me.

[Edit]

But had the shared_ptr been constant, then foobar() would not change the pointer, meaning that the function couldn't do anything bad.

it works very well for that, but as you pointed out, this thread is all about convenience.

O_o

I've added an example showing exactly how the implicit constructor of a resource owning object combines with overload resolution for just absolute totally abysmal code.
[/Edit]

for a smart pointer type that doesn't need to share control with other instances, exists only for the duration of the scope in which it was created, and doesn't do any "fancy stuff," a couple of hours is pretty generous.

I was under the impression you still wanted sharing; that's my bad.

We spell "that" as `std::unique_ptr' or some similar variation such as `boost::scoped_ptr'.

Soma

Code:

// THIS CODE IS BAD!
// I DO NOT SUGGEST ANYONE WRITE SUCH CODE!
// I POSTED IT SPECIFICALLY TO SHOW THE FLAWS!

#include <iostream>
#include <memory>

template
<
    typename FType
>
struct SSuperAwfulPointer
{
    SSuperAwfulPointer
    (
        FType * f
    ): m(f)
    {
    }
    ~SSuperAwfulPointer()
    {
        delete m;
    }
    FType * m;
};

struct STest
{
    STest()
    {
        std::cout << "\tSTest()" << std::endl;
    }
    STest
    (
        const STest &
    )
    {
        std::cout << "\tSTest(const &)" << std::endl;
    }
    ~STest()
    {
        std::cout << "\t~STest()" << std::endl;
    }
    STest & operator =
    (
        const STest &
    )
    {
        std::cout << "\tSTest & operator = (const &)" << std::endl;
        return(*this);
    }
    STest * go() // This is NOT a constant method.
    {
        return(this);
    }
};

void foobar1(const std::shared_ptr<STest> & p){}
void foobar2(const SSuperAwfulPointer<STest> & p){}

void Go1()
{
    std::cout << "Go1-STD-Begin" << std::endl;
    STest * s(new STest);               // The temporary variable is not a constant object within this frame of
    foobar1(std::shared_ptr<STest>(s));  // consideration; the temprary can only bind to a constant reference.
    // delete s; // This is a "double deletion" error.
    std::cout << "Go1-STD-End" << std::endl;
}

void Go2()
{
    std::cout << "Go2-STD-Begin" << std::endl;
    { // This is just to force a particular unwinding.
        STest * s(new STest);
        std::shared_ptr<STest> sP(s);
        foobar1(sP);
        // delete s; // This is a "double deletion" error.
    }
    std::cout << "Go2-STD-End" << std::endl;
}

void Go3()
{
    // This exists only to show how the non-constant method is called
    // from the frame of reference of the temporary object.
    std::cout << "Go3-STD-Begin" << std::endl;
    foobar1(std::shared_ptr<STest>((new STest)->go()));
    std::cout << "Go3-STD-End" << std::endl;
    // This works for the same reason
    // the "swap trick works":
    // std::vector<Type>().swap(sTargetVector);
}

void Go4()
{
    std::cout << "Go4-SAP-Begin" << std::endl;
    STest * s(new STest);               // The temporary variable is not a constant object within this frame of
    foobar2(s);                         // consideration; the temprary can only bind to a constant reference.
    // delete s; // This is a "double deletion" error.
    std::cout << "Go4-SAP-End" << std::endl;
}

void Go5()
{
    std::cout << "Go5-SAP-Begin" << std::endl;
    foobar2(new STest); // This is just awful!
    // delete s; // This is a "double deletion" error.
    std::cout << "Go5-SAP-End" << std::endl;
}

int main()
{
    Go1();
    Go2();
    Go3();
    Go4();
    Go5();
    return(0);
}

Originally Posted by phantomotap

We spell "that" as `std::unique_ptr' or some similar variation such as `boost::scoped_ptr'.

unfortunately, neither of them allow implicit construction either.