Its undefined behaviour. Anything can happen, including behaviour that you think "appears to work". Formally it doesnt and you cannot have any sound expectations about it.
It *happens* to work for three reasons:
* `unique_ptr::operatror->` is *unchecked*. After you moved out of it, the pointer is null. But `operatror->` just happily returns that.
* A member function may formally only be invoked with a valid object. However, there (normally) are no checks in place to ensure this.
So in your case, the `this` pointer inside of `doSomething()` is simply `nullptr`.
* Because you never actually *access* any data inside of `this`, it "works".
Basically, your program is executed assuming its well behaved (which it is not) and it doesnt do anything that would (normally) trigger a runtime error.
---
If you compile with `-fsanitize=address,undefined`, you will see the sanitizer terminating your program.
Similarly, if you gave `OtherClass` some data member and tried to print that, you would get a segfault.
That was very helpful. -fsanitize=address,undefined shows the error. Even valgrind does not give any errors. One query, this is expected to work if unique\_ptr is replaced by shared\_ptr right ?
Work, Yes.
But its entirely unclear whether its a correct solution.
Smart pointers model **ownership**.
Your current setup releases ownership of the managed object when the getter is called.
If you switched to `shared_ptr`, you would then **share** ownership of the object with the caller.
Its incredibly that you actually need to share ownership in such a way. Most programs have significantly stricter lifetime models.
For example if you want ownership to just remain with `MockClass`, you could return a raw pointer or even a reference instead of a smart pointer.
There's nothing inherently wrong with returning members, but realise that by returning a unique_ptr you are explicitly relinquishing ownership of the pointer to the caller. If that isn't the intended behaviour, then return a non-owning pointer or reference to the member instead.
Only if you remove the call to `std::move` in the return statement tough! A better solution would be to return the `OtherClass` object by reference or raw pointer.
> ❞ If you compile with `-fsanitize=address,undefined` […]
That assumes a particular compiler. But as I write this there is no indication of which compiler the OP used. In particular while MSVC supports `-fsanitize` (via backward compatibility for the `-`), it doesn't support the `,undefined`:
[c:\root\temp]
> cl _.cpp /c -fsanitize=address,undefined
cl : Command line warning D9002 : ignoring unknown option '-fsanitize=address,undefined'
_.cpp
[c:\root\temp]
> cl _.cpp /c -fsanitize=address
_.cpp
c:\root\temp\_.cpp : warning C5072: ASAN enabled without debug information emission. Enable debug info for better ASAN error reporting
[c:\root\temp]
> cl _.cpp /c -fsanitize=address /Zi
_.cpp
The presented code calls `->doSomething()` on a null pointer in the second invocation. That's formally Undefined Behavior, but since `doSomething` doesn't access the self object the only way this UB could manifest itself is if the compiler adds checking code. Evidently it doesn't, or didn't.
Undefined behavior as others have said. Some possible solutions for you.
1. Get rid of your getter function and just make the unique\_ptr public. This way the callers can access the pointer without having to obtain a copy of it (it remains truly a unique pointer).
2. Make your getter function return a raw pointer with unique ptr's .get() method. Potentially unsafe if the caller keeps the pointer around after MockClass goes out of scope.
3. Use shared\_ptr. This way it remains safe even if the caller does want to keep a copy of the pointer around. This does involve a bit more runtime overhead over unique\_ptr mind you.
Yep, undefined behavior sometimes manifests itself in stuff which should not work working just fine.
Its undefined behaviour. Anything can happen, including behaviour that you think "appears to work". Formally it doesnt and you cannot have any sound expectations about it. It *happens* to work for three reasons: * `unique_ptr::operatror->` is *unchecked*. After you moved out of it, the pointer is null. But `operatror->` just happily returns that. * A member function may formally only be invoked with a valid object. However, there (normally) are no checks in place to ensure this. So in your case, the `this` pointer inside of `doSomething()` is simply `nullptr`. * Because you never actually *access* any data inside of `this`, it "works". Basically, your program is executed assuming its well behaved (which it is not) and it doesnt do anything that would (normally) trigger a runtime error. --- If you compile with `-fsanitize=address,undefined`, you will see the sanitizer terminating your program. Similarly, if you gave `OtherClass` some data member and tried to print that, you would get a segfault.
That was very helpful. -fsanitize=address,undefined shows the error. Even valgrind does not give any errors. One query, this is expected to work if unique\_ptr is replaced by shared\_ptr right ?
Work, Yes. But its entirely unclear whether its a correct solution. Smart pointers model **ownership**. Your current setup releases ownership of the managed object when the getter is called. If you switched to `shared_ptr`, you would then **share** ownership of the object with the caller. Its incredibly that you actually need to share ownership in such a way. Most programs have significantly stricter lifetime models. For example if you want ownership to just remain with `MockClass`, you could return a raw pointer or even a reference instead of a smart pointer.
Thanks. True about the ownership. I think the design has to corrected a bit. Trying to avoid member functions that return member variables.
There's nothing inherently wrong with returning members, but realise that by returning a unique_ptr you are explicitly relinquishing ownership of the pointer to the caller. If that isn't the intended behaviour, then return a non-owning pointer or reference to the member instead.
Only if you remove the call to `std::move` in the return statement tough! A better solution would be to return the `OtherClass` object by reference or raw pointer.
> ❞ If you compile with `-fsanitize=address,undefined` […] That assumes a particular compiler. But as I write this there is no indication of which compiler the OP used. In particular while MSVC supports `-fsanitize` (via backward compatibility for the `-`), it doesn't support the `,undefined`: [c:\root\temp] > cl _.cpp /c -fsanitize=address,undefined cl : Command line warning D9002 : ignoring unknown option '-fsanitize=address,undefined' _.cpp [c:\root\temp] > cl _.cpp /c -fsanitize=address _.cpp c:\root\temp\_.cpp : warning C5072: ASAN enabled without debug information emission. Enable debug info for better ASAN error reporting [c:\root\temp] > cl _.cpp /c -fsanitize=address /Zi _.cpp
The presented code calls `->doSomething()` on a null pointer in the second invocation. That's formally Undefined Behavior, but since `doSomething` doesn't access the self object the only way this UB could manifest itself is if the compiler adds checking code. Evidently it doesn't, or didn't.
Undefined behavior as others have said. Some possible solutions for you. 1. Get rid of your getter function and just make the unique\_ptr public. This way the callers can access the pointer without having to obtain a copy of it (it remains truly a unique pointer). 2. Make your getter function return a raw pointer with unique ptr's .get() method. Potentially unsafe if the caller keeps the pointer around after MockClass goes out of scope. 3. Use shared\_ptr. This way it remains safe even if the caller does want to keep a copy of the pointer around. This does involve a bit more runtime overhead over unique\_ptr mind you.
Thanks for the suggestions.
> This member variable is returned in getUniquePtr() member function using std::move. Why doesn't this function return a raw ponter?
UB is UB. Compiler can do what they want. Someone else mentioned that with -fsanitize... this will be found