9

I was developing some class and bumped for this question. Consider I have following class:

struct A
{
    int *p;
    A() 
    {
        p = new int(1); 
        cout << "ctor A" << endl; 
    }
    A(const A& o) 
    { 
        cout << "copy A" << endl;
        p = new int(*(o.p));
    }
    A(A&& o) 
    {
        cout << "move A" << endl;
        p = std::move(o.p);
        o.p = NULL;
    }
    A& operator=(const A& other)
    {       
        if (p != NULL)
        {
            delete p;
        }
        p = new int(*other.p);
        cout << "copy= A" << endl;
        return *this;
    }
    A& operator=(A&& other)
    {       
        p = std::move(other.p);
        other.p = NULL;
        cout << "move= A" << endl;
        return *this;
    }
    ~A()
    {
        if(p!=NULL)
            delete p;
        p = NULL;
        cout << "dtor A" << endl;
    }
};

And following class which has A as a property:

class B {
public:
  B(){}
  A myList;
  const A& getList() { return myList; };
};

And this function which checks for some variable value and returns different objects in different cases:

B temp;
A foo(bool f)
{
    A a;
    *a.p = 125; 
    if (f)
        return a;
    else
    {
        return temp.getList();
    }
}

Now, I am want to use this function like this:

A list1 = foo(true);
if(list1.p != NULL)
    cout << (*list1.p) << endl;

cout << "------"<<endl;
A list2 = foo(false);
if (list2.p != NULL)
    cout << (*list2.p) << endl;

The purpose of this situation is:

Function foo should return (or move) without copying some local object with changes in p if argument is true, or should return property of global variable temp without calling copy constructors of A (i.e. return reference of myList) and also it should not grab myList from B (it should not destroy myList from B, so std::move can not be used) if argument is false.

My question is:

How should i change function foo to follow upper conditions? Current implementation of foo works right in true case and moving that local variable, but in case false it calls copy constructor for list2. Other idea was to somehow extend lifetime of local variable, but adding const reference did not work for me. Current output is:

ctor A
ctor A
move A
dtor A
125
------
ctor A
copy A
dtor A
1
dtor A
dtor A
dtor A
  • The problem is what you want to return from `foo` is something that might or might not own an `A` and there is no such thing as an optionally-owning smart-pointer but you could probably make one. – Chris Drew Jul 21 '17 at 08:51
  • @ChrisDrew so you think it is impossible to accomplish my conditions for `foo` with only C++'s references mechanics and move semantics? – Egor Schavelev Jul 21 '17 at 08:59
  • 2
    As far as I know, yes, but someone cleverer than me might prove me wrong. I think you could accomplish what you want with a `std::shared_ptr` or your own wrapper that might or might not own an `A`. – Chris Drew Jul 21 '17 at 09:01
  • And I am forced implement and use some custom structures for handling this situation? – Egor Schavelev Jul 21 '17 at 09:01
  • @ChrisDrew anyway, thanks for the answer. – Egor Schavelev Jul 21 '17 at 09:02
  • I think that you need to create a custom class for this, if you need the optimal solution. Here's a related question of mine: https://stackoverflow.com/questions/44597835/c-optimizing-out-destructor-call – geza Jul 21 '17 at 11:36
  • Your copy-ssignment operator is not exception-sfe – M.M Jul 21 '17 at 13:05

3 Answers3

4

If you can change B to

class B {
public:
  B(){}
  std::shared_ptr<A> myList = std::make_shared<A>();
  const std::shared_ptr<A>& getList() const { return myList; };
};

then foo can be:

B b;
std::shared_ptr<A> foo(bool cond)
{
    if (cond) {
        auto a = std::make_shared<A>();
        *a->p = 125; 

        return a;
    } else {
        return b.getList();
    }
}

Demo

Output is

ctor A
ctor A
125
------
1
dtor A
dtor A
Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • Thank you very much for your answer, works like a charm! – Egor Schavelev Jul 21 '17 at 12:20
  • By the way, if someone know certainly whether it is possible to do it without smart pointers but with some move semantics and const referencing feel free to answer, I am interested in learning its mechanics deeper. – Egor Schavelev Jul 21 '17 at 12:28
  • May I disturb you again? I read comments more precisely and now want to try change my definition of `foo`. Is it possible to do what I asked without pointers if `foo` would return const reference for the `A`? For example, I am not going to change `A` further, read-only access now is enough for me. – Egor Schavelev Jul 21 '17 at 12:42
  • @EgorSchavelev: No, it's not safe to return by reference any newly created object. If the object is automatic, it will be destroyed before the return, and the caller will be left with a useless (dangling) reference. If the object is dynamic or added to some global collection, the reference will be valid, but the object will also leak. – Ben Voigt Jul 21 '17 at 14:50
  • @BenVoigt But what about prolonging lifetime of an object? As I said, now I can afford getting const reference. Something like here: https://monoinfinito.wordpress.com/2014/01/07/extending-the-life-of-a-temp-variable-in-c/ – Egor Schavelev Jul 21 '17 at 15:56
  • @Egor: Lifetime extension doesn't apply here. The object is not a temporary, it's a scoped local. And return-by-reference never causes lifetime extension. You have a totally different scenario than the one shown on that blog. – Ben Voigt Jul 21 '17 at 17:15
  • @EgorSchavelev: You can do it without a smart pointer. You can store a flag in A, whether it owns `p` or not. This way, you can create an `A`, which can store a pointer to `b.p`, but doesn't own it. But you must be careful, you must not delete `b.p` afterwards, because `A` will refer to the same memory area. – geza Jul 21 '17 at 17:22
2

The simplest solution is probably to use std::shared_ptr as in Jarod42's answer. But if you want to avoid smart pointers, or if you can't change B you can probably create your own wrapper class that might or might not own an A. std::optional might be quite convenient for this:

class AHolder {
  private:
    std::optional<A> aValue;
    const A& aRef;
  public:
    AHolder(const A& a) : aRef(a) {}
    AHolder(A&& a) : aValue(std::move(a)), aRef(aValue.value()) {}
    const A* operator->() const { return &aRef; }
};

The class contains an optional to own the A if required and you can use move-semantics to move it in. The class also contains a reference (or pointer) that either references the contained value or references another object.

You can return this from foo:

AHolder foo(bool f)
{
    A a;
    *a.p = 125; 
    if (f)
        return a;
    else
    {
        return temp.getList();
    }
}

And the caller can access the contained reference:

  auto list1 = foo(true);
  if(list1->p != nullptr)
    cout << (*list1->p) << endl;

  cout << "------"<<endl;
  auto list2 = foo(false);
  if (list2->p != nullptr)
    cout << *list2->p << endl;

Live demo.

If you don't have access to std::optional there is boost::optional or you could use std::unique_ptr at the cost of a dynamic memory allocation.

Chris Drew
  • 14,926
  • 3
  • 34
  • 54
0

Your function foo returns an instance of A, not a reference (nor a pointer), so you can`t get access to B.myList content without copying or moving.

In order to have this access you should either use smart pointers (like Jarod42 wrote) or just simple pointers like this:

B temp;
A* foo(bool f)
{
    if (f)
    {
        A* ptr = new A;
        *ptr->p = 125;
        return ptr;
    }    
    else
    {
        return &temp.getList();
    }
}

However this particular code will not work coz .getList() returns const reference but foo returns non-const pointer (this could but should not be dirty hacked with const_cast<>).

Generally you need to choose what exactly the foo function should return:

  • new instance of A class with specific data
  • access to existing instance

If you have to make this decision on runtime (for example by your bool parameter) then the pointers (simple or smart - whatever) are the only option (also remember to delete manually allocated memory).

Glib
  • 304
  • 1
  • 13
  • 1
    This is a sure way to introduce a memory leak, unless all callers know that calling foo(true) returns a freshly allocated A and foo(false) returns a pointer to a statically owned A. – PaulR Jul 21 '17 at 12:07
  • It is. Yet having a function that returns references (no meter in what way) and instances depending on input is already a possible memory leak. – Glib Jul 21 '17 at 12:13
  • Thanks for your answer, but I was interested to do it without managing memory and preferably using move semantics and some features of C++11/14. – Egor Schavelev Jul 21 '17 at 12:31
  • Than you have to work with references. In such case you need to either return const reference for both local *A* and *myList* or const_cast one of them if you are sure in does not crush the program. The most suitable solution depends on what are you going to do with returned values - just observe or write somewhere or else... – Glib Jul 21 '17 at 13:06
  • @Gleb: Your comment appears to advocate returning a dangling reference :( – Ben Voigt Jul 21 '17 at 14:52