Good catch, and I think it would be good to do something to prevent this. Few ideas:

Result class could drop operator bool and force you to write if (result.has_value()) instead of if (result). I don't thnk this would really solve problem because Result<unique_ptr> could still be assigned nullptr, and result.has_value() would still return true in that case.
Return type could be changed from Result<unique_ptr> to Result<not_null<unique_ptr>> or Result<nn_unique_ptr> using a not-null helper class like one from https://github.com/bitwizeshift/not_null#references. I think this would be a potentially good solution because it would make it a compile time error to assign a nullptr or plain unique_ptr to the result. Only a non-nullable pointer could be assigned to avoid the compiler error.
Could introduce util::NullableResult to complement util::Result. Nullable result class would have the same behavior as normal result class except operator bool would be implemented as return has_value() && value() instead of return has_value(). The name NullableResult would clearly communicate that a null result is possible and not an error, and bool operator would make it easier to check for.
Could do nothing. Result<unique_ptr>{nullptr} being true is not any different from optional<unique_ptr>{nullptr} being true or optional<int>{0} being true or const char* s{""} being true. It's kind of just how indirection works in C/C++ with multiple layers of indirection.

I tend to think (2) is nicest solution, but I'm not sure if there are other use-cases for non-nullable pointers that would justify the complexity since they don't seem completely trivial to implement. I don't like solution (1) because it creates an unnecessary difference between std::optional and util::Result and IMO would makes code using the result class look ugly. Solution (3) I think could be implemented in a few lines and I think would be better than the status quo, because it communicates that the value can be null and makes it easy to check for, even if it doesn't prevent null from being assigned.

util::Result has confusing interface for std::*_ptr T #26004

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions