unsafe global_ptr bool conversion
Our implementation of global_ptr
currently contains the following member function:
operator bool() const {
return raw_ptr_ != nullptr;
}
This enables the end-user to write handy things like the following, which people generally expect from a pointer-like class:
global_ptr<int> gp = upcxx::new_<int>();
if (gp) cout << "true" << endl;
if (!gp) cout << "false" << endl;
else cout << "true" << endl;
This usage pattern is not currently required to work by the specification, but it seems a sufficiently useful pattern that we should consider guaranteeing it will work.
Unfortunately, the use of operator bool()
is a low-quality implementation of this idiom. In particular, it allows all the following surprising/erroneous behavior that we really don't want:
int x = gp; // should really be a type error, but is not!
gp << 1; // this should also be a type error, but is not!
// this should really be a type error.
// instead, it not only compiles but the equality test passes!!!
global_ptr<double> gpd = upcxx::new_<double>();
if (gp == gpd) cout << "OOPS" << endl;
We really want the compiler to reject all the code above, but it does not (tested with gcc 5.4.0 and 8.2.0).
The comparison in the last line is especially problematic: it not only compiles without complaint, but gives a very unintuitive result - the comparison appears to be testing pointer equality, but the condition it's actually testing is whether both pointers are non-null (or both are null). The problem is fairly obvious in this tiny snippet, but when the pointer declarations are in separate files this could be a very difficult bug to track down.
This defect is present in both the develop head and the current public release.
Luckily, there is a better way to accomplish what we want here: The Safe Bool Idiom
it's specifically designed to provide the boolean testing idiom without introducing the problems described above.
We should investigate adapting the Safe Bool idiom to global_ptr
(with minor modifications, since we need correctly-typed pointer comparisons to work).
Comments (3)
-
-
reporter This conversion isn't in the spec; do we want it there?
I would very much like to see the boolean idiom specced (and I suspect there is already code relying upon it), although I'm not sure the spec should mandate an implementation.
-
- changed status to resolved
- Log in to comment
The C++11 solution is to make the operator
explicit
.This conversion isn't in the spec; do we want it there?