IntrusivePtr: replace the "add_ref" parameter with tag structs

Using a runtime parameter is obscure and error-prone.  Avoiding
error-prone code and getting reference counting right is the whole
point of this class.
This commit is contained in:
Max Kellermann 2020-02-19 05:48:02 +01:00
parent 7c0863dccf
commit 31b3a56740
7 changed files with 41 additions and 15 deletions

View file

@ -2828,7 +2828,7 @@ Val* IndexExpr::Fold(Val* v1, Val* v2) const
void IndexExpr::Assign(Frame* f, Val* arg_v) void IndexExpr::Assign(Frame* f, Val* arg_v)
{ {
IntrusivePtr v{arg_v, false}; IntrusivePtr v{AdoptRef{}, arg_v};
if ( IsError() ) if ( IsError() )
return; return;
@ -2849,7 +2849,7 @@ void IndexExpr::Assign(Frame* f, Val* arg_v)
// Hold an extra reference to 'arg_v' in case the ownership transfer to // Hold an extra reference to 'arg_v' in case the ownership transfer to
// the table/vector goes wrong and we still want to obtain diagnostic info // the table/vector goes wrong and we still want to obtain diagnostic info
// from the original value after the assignment already unref'd. // from the original value after the assignment already unref'd.
IntrusivePtr v_extra{arg_v, true}; IntrusivePtr v_extra{NewRef{}, arg_v};
switch ( v1->Type()->Tag() ) { switch ( v1->Type()->Tag() ) {
case TYPE_VECTOR: case TYPE_VECTOR:

View file

@ -5,6 +5,18 @@
#include <type_traits> #include <type_traits>
#include <utility> #include <utility>
/**
* A tag class for the #IntrusivePtr constructor which means: adopt
* the reference from the caller.
*/
struct AdoptRef {};
/**
* A tag class for the #IntrusivePtr constructor which means: create a
* new reference to the object.
*/
struct NewRef {};
/** /**
* An intrusive, reference counting smart pointer implementation. Much like * An intrusive, reference counting smart pointer implementation. Much like
* @c std::shared_ptr, this smart pointer models shared ownership of an object * @c std::shared_ptr, this smart pointer models shared ownership of an object
@ -54,13 +66,27 @@ public:
/** /**
* Constructs a new intrusive pointer for managing the lifetime of the object * Constructs a new intrusive pointer for managing the lifetime of the object
* pointed to by @c raw_ptr. * pointed to by @c raw_ptr.
*
* This overload adopts the existing reference from the caller.
*
* @param raw_ptr Pointer to the shared object. * @param raw_ptr Pointer to the shared object.
* @param add_ref Denotes whether the reference count of the object shall be
* increased during construction.
*/ */
IntrusivePtr(pointer raw_ptr, bool add_ref) noexcept IntrusivePtr(AdoptRef, pointer raw_ptr) noexcept
{ {
setPtr(raw_ptr, add_ref); setPtr(raw_ptr, false);
}
/**
* Constructs a new intrusive pointer for managing the lifetime of the object
* pointed to by @c raw_ptr.
*
* This overload adds a new reference.
*
* @param raw_ptr Pointer to the shared object.
*/
IntrusivePtr(NewRef, pointer raw_ptr) noexcept
{
setPtr(raw_ptr, true);
} }
IntrusivePtr(IntrusivePtr&& other) noexcept : ptr_(other.release()) IntrusivePtr(IntrusivePtr&& other) noexcept : ptr_(other.release())
@ -69,8 +95,8 @@ public:
} }
IntrusivePtr(const IntrusivePtr& other) noexcept IntrusivePtr(const IntrusivePtr& other) noexcept
: IntrusivePtr(NewRef{}, other.get())
{ {
setPtr(other.get(), true);
} }
template <class U, class = std::enable_if_t<std::is_convertible_v<U*, T*>>> template <class U, class = std::enable_if_t<std::is_convertible_v<U*, T*>>>
@ -157,7 +183,7 @@ template <class T, class... Ts>
IntrusivePtr<T> make_intrusive(Ts&&... args) IntrusivePtr<T> make_intrusive(Ts&&... args)
{ {
// Assumes that objects start with a reference count of 1! // Assumes that objects start with a reference count of 1!
return {new T(std::forward<Ts>(args)...), false}; return {AdoptRef{}, new T(std::forward<Ts>(args)...)};
} }
// -- comparison to nullptr ---------------------------------------------------- // -- comparison to nullptr ----------------------------------------------------

View file

@ -207,7 +207,7 @@ static IntrusivePtr<EnumVal> lookup_enum_val(const char* module_name, const char
int index = et->Lookup(module_name, name); int index = et->Lookup(module_name, name);
assert(index >= 0); assert(index >= 0);
IntrusivePtr<EnumVal> rval{et->GetVal(index), false}; IntrusivePtr<EnumVal> rval{AdoptRef{}, et->GetVal(index)};
return rval; return rval;
} }

View file

@ -1984,7 +1984,7 @@ void TableVal::CallChangeFunc(const Val* index, Val* old_value, OnChangeType tpe
try try
{ {
IntrusivePtr<Val> thefunc{change_func->Eval(nullptr), false}; IntrusivePtr<Val> thefunc{AdoptRef{}, change_func->Eval(nullptr)};
if ( ! thefunc ) if ( ! thefunc )
{ {

View file

@ -792,9 +792,9 @@ static bool data_type_check(const broker::data& d, BroType* t)
IntrusivePtr<Val> bro_broker::data_to_val(broker::data d, BroType* type) IntrusivePtr<Val> bro_broker::data_to_val(broker::data d, BroType* type)
{ {
if ( type->Tag() == TYPE_ANY ) if ( type->Tag() == TYPE_ANY )
return {bro_broker::make_data_val(move(d)), false}; return {AdoptRef{}, bro_broker::make_data_val(move(d))};
return {caf::visit(val_converter{type}, std::move(d)), false}; return {AdoptRef{}, caf::visit(val_converter{type}, std::move(d))};
} }
broker::expected<broker::data> bro_broker::val_to_data(const Val* v) broker::expected<broker::data> bro_broker::val_to_data(const Val* v)

View file

@ -1024,7 +1024,7 @@ Supervisor::NodeConfig Supervisor::NodeConfig::FromRecord(const RecordVal* node)
while ( (v = cluster_table->NextEntry(k, c)) ) while ( (v = cluster_table->NextEntry(k, c)) )
{ {
IntrusivePtr<ListVal> key{cluster_table_val->RecoverIndex(k), false}; IntrusivePtr<ListVal> key{AdoptRef{}, cluster_table_val->RecoverIndex(k)};
delete k; delete k;
auto name = key->Index(0)->AsStringVal()->ToStdString(); auto name = key->Index(0)->AsStringVal()->ToStdString();
auto rv = v->Value()->AsRecordVal(); auto rv = v->Value()->AsRecordVal();
@ -1100,7 +1100,7 @@ std::string Supervisor::NodeConfig::ToJSON() const
{ {
auto re = std::make_unique<RE_Matcher>("^_"); auto re = std::make_unique<RE_Matcher>("^_");
auto node_val = ToRecord(); auto node_val = ToRecord();
IntrusivePtr<StringVal> json_val{node_val->ToJSON(false, re.get()), false}; IntrusivePtr<StringVal> json_val{AdoptRef{}, node_val->ToJSON(false, re.get())};
auto rval = json_val->ToStdString(); auto rval = json_val->ToStdString();
return rval; return rval;
} }

View file

@ -400,7 +400,7 @@ function terminate%(%): bool
// is false). // is false).
static bool prepare_environment(TableVal* tbl, bool set) static bool prepare_environment(TableVal* tbl, bool set)
{ {
IntrusivePtr<ListVal> idxs{tbl->ConvertToPureList(), false}; IntrusivePtr<ListVal> idxs{AdoptRef{}, tbl->ConvertToPureList()};
for ( int i = 0; i < idxs->Length(); ++i ) for ( int i = 0; i < idxs->Length(); ++i )
{ {