mapbox / geometry.hpp

C++ geometry types
ISC License
92 stars 37 forks source link

Change std::nullptr_t to explicit null type #33

Closed kkaefer closed 8 years ago

kkaefer commented 8 years ago

Instead of using std::nullptr_t, we should use an explicit null type (like a struct null_value {};). std::string has an overload that also accepts std::nullptr_t.

See https://github.com/mapbox/variant/issues/100

artemp commented 8 years ago

👍

jfirebaugh commented 8 years ago

The issue with this is that value will still be constructible or assignable with nullptr, but instead of being set to the null alternative as you might expect (and as happens now), it will segfault at runtime.

kkaefer commented 8 years ago

It segfaults because the std::string constructor is invoked with the nullptr.

jfirebaugh commented 8 years ago

My bad -- it does not segfault. It constructs a value where bool is the stored type and false is its value.

I would feel more comfortable with this change if we could fix https://github.com/mapbox/variant/issues/100, such that an ambiguous assignment from nullptr was an error.

jfirebaugh commented 8 years ago

I think #40 avoids those potential issues!