Drawback of Declaring and Defining Variable at Return

Question

Is there drawback in declaring and defining variable at return?

Here is one case from the user Man in Black's question. If we return a reference to a local variable, we will end up with an incorrect value. The reason is that the local variable would be destroyed upon returning.

One of solution is using "shared_ptr" like this by the user Christophe:

struct node {
  int data;
  node* next;
};

shared_ptr<node> create_node(int value) { 
    node* newitem = new node;
    newitem->data=value;
    newitem->next=NULL;

    return (shared_ptr<node>(newitem)); 
}

This is a concise solution, but at the returning code line, it declare and define variable of "shared_ptr(newitem)".

This is declaring variable, temp, and then define it:

shared_ptr<node> temp; // Declare variable
temp = shared_ptr<node>(newitem); // Define variable

return temp;

I am trying to avoid memory allocation failure. Would we say this is better in the case of failing memory allocation?

Jolynjolynn

"at the returning code line, it declare and define variable of 'shared_ptr(newitem)'." -- No, a variable has a name. The line return (shared_ptr<node>(newitem)); creates an object of a shared_ptr type, no declaration or definition here. – Gnni

@Gnni would you share some difference between object and variable in C++ language? Is https://mcmap.net/q/261709/-what-is-the-difference-between-a-variable-object-and-reference-duplicate a good explanation? – Jolynjolynn

"Is https://mcmap.net/q/261709/-what-is-the-difference-between-a-variable-object-and-reference-duplicate a good explanation?" -- Yes, that looks good. A simplistic view is to equate "variable" with "has a name". You gave the name newitem to a certain pointer; that's a variable. There is no name associated with the result of shared_ptr<node>(newitem); that's not a variable. – Gnni

New terminology issue: shared_ptr<node> temp; both declares and defines a variable. The line temp = shared_ptr<node>(newitem); assigns a value to (almost initializes) the variable; there is no definition at this point. – Gnni

@JaMit, if shared_ptr<node> temp is also defining, what we will see when we call the "temp"? Isn't random value from unknown memory location? – Jolynjolynn

"if shared_ptr<node> temp is also defining, what we will see [...] -- This question is off-base, because defining a variable does not mean giving it a value; defining a variable means providing space for a value. See What is the difference between a definition and a declaration? and its follow-up What distinguishes the declaration, the definition and the initialization of a variable? – Gnni

@Gnni You may be confused. I am asking for the variable. "A constant variable must be defined, in other words assigned a value, in the same statement in which it's declared." from learn.microsoft.com/en-us/cpp/cpp/…. – Jolynjolynn

"from learn.microsoft.com/en-us/cpp/cpp/…" -- Microsoft uses the terminology incorrectly. A const variable must be initialized, in other words assigned a value, in the same statement in which it's defined. (The sentence after that one is also incorrect. A declaration of a built-in type such as int is not automatically a definition. The declaration extern int i; is not a definition.) – Gnni

@Gnni Umm...How about this link, linuxhint.com/declare-and-define-variables-c? Initialization sounds like declaration and defining according to Hiba Shafoat, an author of this page in Linux Hint. – Jolynjolynn

Sorry, I am not interested in debunking every link you come up with. I gave you two links to answered Stack Overflow questions that cover this. If you want an outside reference, there's cppreference.com, which can be a rough read, but tends to be accurate: Declarations, Definitions, and Initialization. If that's not enough, start a new question; comments are not for extended discussions. – Gnni

Answer 1

shared_ptr<node> create_node(int value) { 
    node* newitem = new node;
    newitem->data=value;
    newitem->next=NULL;

    return (shared_ptr<node>(newitem)); 
}

Has three issues:

1. (Mostly theoretical, the code as written is safe): If an exception can be thrown between the new and the wrapping in a shared_ptr, the unmanaged pointer will be leaked.
2. (More practical) By allocating the pointer separately from creating the shared_ptr, you fragment your memory more (the shared_ptr constructor must allocate a control block separately for reference counting and managing the owned pointer, and therefore access to the object through the shared_ptr involves a double-indirection). This does mean that, if weak_ptrs are involved, and all the shared_ptrs stop existing, it can release the object memory, and only preserve the control block for the weak_ptrs, saving memory if many such objects are reduced to that state, but it slows everything else down a little bit to achieve that result.
3. (Maybe) If your node is constructable, you should really set these values by constructing it directly, rather than creating it uninitialized and filling in the values manually, to simplify your code if nothing else (if it doesn't have a constructor, this isn't an issue).

If it has a constructor, e.g. one that receives data and leaves the next node initialized to NULL, this is ideal:

shared_ptr<node> create_node(int value) {
    return std::make_shared<node>(value);
}

It allocates both tracking metadata and the object in one block, initializes it directly, and returns it without involving a named variable, so guaranteed copy-elision applies in C++17 and later (which means you won't be doing atomic reference count manipulations or the like when you're really just handing off ownership).

If a constructor doesn't/can't exist, the next best solution is:

shared_ptr<node> create_node(int value) { 
    auto newitem = std::make_shared<node>();
    newitem->data = value;  // No need to assign next, make_shared zero-initialized it
    return newitem;
}

This doesn't get guaranteed copy elision, and it does unnecessarily zero-init data when data will be set momentarily, but most major compilers can handle NRVO copy elision just fine in simple cases like this, and the cost of the unnecessary zero-init is trivial next to the benefits you get from std::make_shared (e.g. guaranteeed cleanup if an exception is thrown at any point, not just after wrapping in shared_ptr at some future point, reduced memory fragmentation, more concise code, etc.).

Goldsberry

Please, add some reference of "the unmanaged pointer". Also regarding "guaranteed copy-elision applies in C++17", I ran with C++11, so ... – Jolynjolynn

@CloudCho: "The unmanaged pointer" is one produced directly by new, that has not yet been wrapped in a smart pointer class of some kind (e.g. std::shared_ptr, std::unique_ptr). Even before C++17, copy elision was commonly supported, it just wasn't mandated by the standard. Unnamed values have always been easier to elide, and so on a most compiler from the last decade, I'd expect it to work, you just can't write code that relies on it (no code really should, it would just run slower without it). – Goldsberry

Answer 2

Is there drawback in declaring and defining variable at return?

NO!

I guess the source of your doubt is because you are mixing the concept of "raw pointers" versus "smart pointer" (high level objects).

The smart pointers will be created and the compiler will be able to resolve (in compile-time) the "solution" on how to return the high-level object. Usually in this case, it will probably perform a "move" operation such that this becomes very close to zero-cost. So, you did right.

Usually, behind the scenes, the compiler is able to optimize even more and write the data in your original variable, in the previous stack. But this is tricky and implementation specific. Don't worry about that. The compiler is very smart about this kind of stuff.

The only thing you can't return is a POINTER to a local variable in the stack. Everything else is safe to be done. :-)

Angloirish

Would you share some reference of "smart pointer"? Is this related to automatic memory release? Also "Everything" seems overly simple... ;) – Jolynjolynn

Answer 3

Copy Elision

Is there drawback in declaring and defining variable at return?

No, there isn't. In fact, if you create an object in a return statement, the expression is a prvalue, and copy elision takes place. As a result, this new object is the result object outside the function.

I.e. in

return (shared_ptr<node>(newitem));

... this new shared_ptr<node> object is the same object as the result outside the function.

Exception Safety

That being said, the way that resource management works in your function isn't great:

shared_ptr<node> create_node(int value) { 
    node* newitem = new node;
    newitem->data = value;
    newitem->next = NULL;

    return (shared_ptr<node>(newitem)); 
}

If the assignments or the creation of the shared_ptr throw, the newitem gets leaked. The ideal way to write it is with make_shared:

shared_ptr<node> create_node(int value) { 
    // assuming node has a constructor or is an aggregate type
    return make_shared<node>(value, nullptr);
    // alternatively, for aggregate types pre-C++20
    return make_shared<node>(node{value, nullptr});
}

That being said, since the function is now a one-liner, it's kind of pointless and you could just call make_shared<node> or make_unique<node> directly.

Alternatives

shared_ptr<node> temp;
temp = shared_ptr<node>(newitem);

return temp;

I am trying to avoid memory allocation failure. Would we say this is better in the case of failing memory allocation?

It is strictly worse than your original example.

• you no longer have gauranteed copy elision because temp is an lvalue
• you default-initialize temp and then assign it, which is an anti-pattern
  • see ES.22: Don’t declare a variable until you have a value to initialize it with

Midrib

Please add some explanation or reference at "you default-initialize temp and then assign it, which is an anti-pattern". First time to heard the anti pattern. – Jolynjolynn

@CloudCho CppCoreGuidelines advises against it, among many other people. It's mostly just common sense that calling the constructor when needed is better than calling the default constructor and then assignment operator. It's better for const-correctness and performance. – Midrib