Deprecate ID::ID_Val(), replace with ID::GetVal()

NEWS

@@ -157,6 +157,8 @@ Deprecated Functionality
 
 - ``ID::Type()`` is deprecated, use ``ID::GetType()``.
 
+- ``ID::ID_Val()`` is deprecated, use ``ID::GetVal()``.
+
 Zeek 3.1.0
 ==========
 

src/Debug.cc

@@ -221,7 +221,7 @@ static void parse_function_name(vector<ParseLocationRec>& result,
 		return;
 		}
 
-	const Func* func = id->ID_Val()->AsFunc();
+	const Func* func = id->GetVal()->AsFunc();
 	const vector<Func::Body>& bodies = func->GetBodies();
 
 	if ( bodies.size() == 0 )

src/Expr.cc

@@ -236,7 +236,7 @@ IntrusivePtr<Val> NameExpr::Eval(Frame* f) const
 		return make_intrusive<Val>(id->GetType(), true);
 
 	if ( id->IsGlobal() )
-		v = {NewRef{}, id->ID_Val()};
+		v = id->GetVal();
 
 	else if ( f )
 		v = {NewRef{}, f->GetElement(id.get())};

src/ID.cc

@@ -25,7 +25,6 @@ ID::ID(const char* arg_name, IDScope arg_scope, bool arg_is_export)
 	scope = arg_scope;
 	is_export = arg_is_export;
 	is_option = false;
-	val = nullptr;
 	is_const = false;
 	is_enum_const = false;
 	is_type = false;
@@ -41,8 +40,8 @@ ID::~ID()
 	{
 	delete [] name;
 
-	if ( ! weak_ref )
-		Unref(val);
+	if ( weak_ref )
+		val.release();
 	}
 
 std::string ID::ModuleName() const
@@ -57,18 +56,16 @@ void ID::SetType(IntrusivePtr<BroType> t)
 
 void ID::ClearVal()
 	{
-	if ( ! weak_ref )
-		Unref(val);
-
-	val = nullptr;
+	if ( weak_ref )
+		val.release();
 	}
 
 void ID::SetVal(IntrusivePtr<Val> v, bool arg_weak_ref)
 	{
-	if ( ! weak_ref )
-		Unref(val);
+	if ( weak_ref )
+		val.release();
 
-	val = v.release();
+	val = std::move(v);
 	weak_ref = arg_weak_ref;
 	Modified();
 
@@ -124,12 +121,12 @@ void ID::SetVal(IntrusivePtr<Val> v, init_class c)
 				return;
 				}
 			else
-				v->AddTo(val, false);
+				v->AddTo(val.get(), false);
 			}
 		else
 			{
 			if ( val )
-				v->RemoveFrom(val);
+				v->RemoveFrom(val.get());
 			}
 		}
 	}
@@ -267,7 +264,7 @@ void ID::SetOption()
 
 void ID::EvalFunc(IntrusivePtr<Expr> ef, IntrusivePtr<Expr> ev)
 	{
-	auto arg1 = make_intrusive<ConstExpr>(IntrusivePtr{NewRef{}, val});
+	auto arg1 = make_intrusive<ConstExpr>(val);
 	auto args = make_intrusive<ListExpr>();
 	args->Append(std::move(arg1));
 	args->Append(std::move(ev));
@@ -488,10 +485,8 @@ void ID::DescribeReST(ODesc* d, bool roles_only) const
 		d->Add(":Default:");
 		auto ii = zeekygen_mgr->GetIdentifierInfo(Name());
 		auto redefs = ii->GetRedefs();
-		auto iv = val;
-
-		if ( ! redefs.empty() && ii->InitialVal() )
-			iv = ii->InitialVal();
+		const auto& iv = ! redefs.empty() && ii->InitialVal() ? ii->InitialVal()
+			                                                  : val;
 
 		if ( type->InternalType() == TYPE_INTERNAL_OTHER )
 			{

src/ID.h

@@ -70,8 +70,15 @@ public:
 	void SetVal(IntrusivePtr<Expr> ev, init_class c);
 
 	bool HasVal() const		{ return val != nullptr; }
-	Val* ID_Val()			{ return val; }
-	const Val* ID_Val() const	{ return val; }
+
+	[[deprecated("Remove in v4.1.  Use GetVal().")]]
+	Val* ID_Val()			{ return val.get(); }
+	[[deprecated("Remove in v4.1.  Use GetVal().")]]
+	const Val* ID_Val() const	{ return val.get(); }
+
+	const IntrusivePtr<Val>& GetVal() const
+		{ return val; }
+
 	void ClearVal();
 
 	void SetConst()			{ is_const = true; }
@@ -139,7 +146,7 @@ protected:
 	IntrusivePtr<BroType> type;
 	bool is_const, is_enum_const, is_type, is_option;
 	int offset;
-	Val* val;
+	IntrusivePtr<Val> val;
 	IntrusivePtr<Attributes> attrs;
 	// contains list of functions that are called when an option changes
 	std::multimap<int, IntrusivePtr<Func>> option_handlers;

src/RuleCondition.cc

@@ -164,7 +164,7 @@ bool RuleConditionEval::DoMatch(Rule* rule, RuleEndpointState* state,
 		}
 
 	if ( id->GetType()->Tag() != TYPE_FUNC )
-		return id->ID_Val()->AsBool();
+		return id->GetVal()->AsBool();
 
 	// Call function with a signature_state value as argument.
 	zeek::Args args;
@@ -180,7 +180,7 @@ bool RuleConditionEval::DoMatch(Rule* rule, RuleEndpointState* state,
 
 	try
 		{
-		auto val = id->ID_Val()->AsFunc()->Call(args);
+		auto val = id->GetVal()->AsFunc()->Call(args);
 		result = val && val->AsBool();
 		}
 

src/RuleMatcher.cc

@@ -1278,7 +1278,7 @@ static Val* get_bro_val(const char* label)
 		return nullptr;
 		}
 
-	return id->ID_Val();
+	return id->GetVal().get();
 	}
 
 

src/Stats.cc

@@ -257,9 +257,9 @@ void ProfileLogger::Log()
 			// contained in some other global user-visible container.
 			if ( id->HasVal() )
 				{
-				Val* v = id->ID_Val();
+				const auto& v = id->GetVal();
 
-				size = id->ID_Val()->MemoryAllocation();
+				size = v->MemoryAllocation();
 				mem += size;
 
 				bool print = false;

src/Trigger.cc

@@ -50,7 +50,8 @@ TraversalCode TriggerTraversalCallback::PreExpr(const Expr* expr)
 		if ( e->Id()->IsGlobal() )
 			trigger->Register(e->Id());
 
-		Val* v = e->Id()->ID_Val();
+		Val* v = e->Id()->GetVal().get();
+
 		if ( v && v->Modifiable() )
 			trigger->Register(v);
 		break;

src/Type.cc

@@ -1168,7 +1168,7 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name,
 		// cyclic dependencies.
 		string fullname = make_full_var_name(module_name.c_str(), name);
 		if ( id->Name() != fullname
-		     || (id->HasVal() && val != id->ID_Val()->AsEnum())
+		     || (id->HasVal() && val != id->GetVal()->AsEnum())
 		     || (names.find(fullname) != names.end() && names[fullname] != val) )
 			{
 			reporter->Error("identifier or enumerator value in enumerated type definition already exists");

src/Var.cc

@@ -525,7 +525,7 @@ void begin_func(ID* id, const char* module_name, function_flavor flavor,
 
 	if ( id->HasVal() )
 		{
-		function_flavor id_flavor = id->ID_Val()->AsFunc()->Flavor();
+		function_flavor id_flavor = id->GetVal()->AsFunc()->Flavor();
 
 		if ( id_flavor != flavor )
 			id->Error("inconsistent function flavor", t.get());
@@ -630,7 +630,7 @@ void end_func(IntrusivePtr<Stmt> body)
 	auto ingredients = std::make_unique<function_ingredients>(pop_scope(), std::move(body));
 
 	if ( ingredients->id->HasVal() )
-		ingredients->id->ID_Val()->AsFunc()->AddBody(
+		ingredients->id->GetVal()->AsFunc()->AddBody(
 			ingredients->body,
 			ingredients->inits,
 			ingredients->frame_size,
@@ -648,7 +648,7 @@ void end_func(IntrusivePtr<Stmt> body)
 		ingredients->id->SetConst();
 		}
 
-	ingredients->id->ID_Val()->AsFunc()->SetScope(ingredients->scope);
+	ingredients->id->GetVal()->AsFunc()->SetScope(ingredients->scope);
 	// Note: ideally, something would take ownership of this memory until the
 	// end of script execution, but that's essentially the same as the
 	// lifetime of the process at the moment, so ok to "leak" it.
@@ -662,7 +662,7 @@ Val* internal_val(const char* name)
 	if ( ! id )
 		reporter->InternalError("internal variable %s missing", name);
 
-	return id->ID_Val();
+	return id->GetVal().get();
 	}
 
 id_list gather_outer_ids(Scope* scope, Stmt* body)
@@ -694,13 +694,13 @@ Val* internal_const_val(const char* name)
 	if ( ! id->IsConst() )
 		reporter->InternalError("internal variable %s is not constant", name);
 
-	return id->ID_Val();
+	return id->GetVal().get();
 	}
 
 Val* opt_internal_val(const char* name)
 	{
 	auto id = lookup_ID(name, GLOBAL_MODULE_NAME);
-	return id ? id->ID_Val() : nullptr;
+	return id ? id->GetVal().get() : nullptr;
 	}
 
 double opt_internal_double(const char* name)
@@ -739,7 +739,7 @@ ListVal* internal_list_val(const char* name)
 	if ( ! id )
 		return nullptr;
 
-	Val* v = id->ID_Val();
+	Val* v = id->GetVal().get();
 
 	if ( v )
 		{

src/analyzer/Manager.cc

@@ -92,10 +92,10 @@ void Manager::InitPostScript()
 	{
 	auto id = global_scope()->Lookup("Tunnel::vxlan_ports");
 
-	if ( ! (id && id->ID_Val()) )
+	if ( ! (id && id->GetVal()) )
 		reporter->FatalError("Tunnel::vxlan_ports not defined");
 
-	auto table_val = id->ID_Val()->AsTableVal();
+	auto table_val = id->GetVal()->AsTableVal();
 	auto port_list = table_val->ToPureListVal();
 
 	for ( auto i = 0; i < port_list->Length(); ++i )

src/analyzer/protocol/mqtt/commands/publish.pac

@@ -31,7 +31,7 @@ refine flow MQTT_Flow += {
 			                           reinterpret_cast<const char*>(${msg.topic.str}.begin())));
 
 			auto len = ${msg.payload}.length();
-			auto max = analyzer::MQTT::MQTT_Analyzer::max_payload_size->ID_Val()->AsCount();
+			auto max = analyzer::MQTT::MQTT_Analyzer::max_payload_size->GetVal()->AsCount();
 
 			if ( len > static_cast<int>(max) )
 				len = max;

src/broker/Data.cc

@@ -357,7 +357,7 @@ struct val_converter {
 			if ( ! id )
 				return nullptr;
 
-			auto rval = id->ID_Val();
+			const auto& rval = id->GetVal();
 			if ( ! rval )
 				return nullptr;
 
@@ -701,7 +701,7 @@ struct type_checker {
 			if ( ! id )
 				return false;
 
-			auto rval = id->ID_Val();
+			const auto& rval = id->GetVal();
 			if ( ! rval )
 				return false;
 

src/broker/Manager.cc

@@ -31,10 +31,10 @@ static inline Val* get_option(const char* option)
 	{
 	auto id = global_scope()->Lookup(option);
 
-	if ( ! (id && id->ID_Val()) )
+	if ( ! (id && id->GetVal()) )
 		reporter->FatalError("Unknown Broker option %s", option);
 
-	return id->ID_Val();
+	return id->GetVal().get();
 	}
 
 class BrokerConfig : public broker::configuration {
@@ -418,14 +418,14 @@ bool Manager::PublishIdentifier(std::string topic, std::string id)
 	if ( ! i )
 		return false;
 
-	auto val = i->ID_Val();
+	const auto& val = i->GetVal();
 
 	if ( ! val )
 		// Probably could have a special case to also unset the value on the
 		// receiving side, but not sure what use that would be.
 		return false;
 
-	auto data = val_to_data(val);
+	auto data = val_to_data(val.get());
 
 	if ( ! data )
 		{

src/broker/messaging.bif

@@ -185,7 +185,7 @@ function Cluster::publish_rr%(pool: Pool, key: string, ...%): bool
 	static Func* topic_func = 0;
 
 	if ( ! topic_func )
-		topic_func = global_scope()->Lookup("Cluster::rr_topic")->ID_Val()->AsFunc();
+		topic_func = global_scope()->Lookup("Cluster::rr_topic")->GetVal()->AsFunc();
 
 	zeek::Args vl{{NewRef{}, pool}, {NewRef{}, key}};
 	auto topic = topic_func->Call(vl);
@@ -222,7 +222,7 @@ function Cluster::publish_hrw%(pool: Pool, key: any, ...%): bool
 	static Func* topic_func = 0;
 
 	if ( ! topic_func )
-		topic_func = global_scope()->Lookup("Cluster::hrw_topic")->ID_Val()->AsFunc();
+		topic_func = global_scope()->Lookup("Cluster::hrw_topic")->GetVal()->AsFunc();
 
 	zeek::Args vl{{NewRef{}, pool}, {NewRef{}, key}};
 	auto topic = topic_func->Call(vl);

src/logging/Manager.cc

@@ -1184,7 +1184,7 @@ WriterFrontend* Manager::CreateWriter(EnumVal* id, EnumVal* writer, WriterBacken
 		{
 		ID* id = global_scope()->Lookup("Log::default_rotation_interval");
 		assert(id);
-		winfo->interval = id->ID_Val()->AsInterval();
+		winfo->interval = id->GetVal()->AsInterval();
 		}
 
 	stream->writers.insert(
@@ -1527,7 +1527,7 @@ bool Manager::FinishedRotation(WriterFrontend* writer, const char* new_name, con
 		{
 		ID* id = global_scope()->Lookup("Log::__default_rotation_postprocessor");
 		assert(id);
-		func = id->ID_Val()->AsFunc();
+		func = id->GetVal()->AsFunc();
 		}
 
 	assert(func);

src/option.bif

@@ -104,7 +104,7 @@ function Option::set%(ID: string, val: any, location: string &default=""%): bool
 			// Just coerce an empty/unspecified table to the right type.
 			auto tv = make_intrusive<TableVal>(
 			        cast_intrusive<TableType>(i->GetType()),
-			        IntrusivePtr{NewRef{}, i->ID_Val()->AsTableVal()->Attrs()});
+			        IntrusivePtr{NewRef{}, i->GetVal()->AsTableVal()->Attrs()});
 			auto rval = call_option_handlers_and_set_value(ID, i, std::move(tv), location);
 			return val_mgr->Bool(rval);
 			}

src/supervisor/Supervisor.cc

@@ -1195,7 +1195,7 @@ bool Supervisor::SupervisedNode::InitCluster() const
 	auto cluster_node_type = global_scope()->Lookup("Cluster::Node")->GetType()->AsRecordType();
 	auto cluster_nodes_id = global_scope()->Lookup("Cluster::nodes");
 	auto cluster_manager_is_logger_id = global_scope()->Lookup("Cluster::manager_is_logger");
-	auto cluster_nodes = cluster_nodes_id->ID_Val()->AsTableVal();
+	auto cluster_nodes = cluster_nodes_id->GetVal()->AsTableVal();
 	auto has_logger = false;
 	std::optional<std::string> manager_name;
 

src/zeek-setup.cc

@@ -631,11 +631,11 @@ zeek::detail::SetupResult zeek::detail::setup(int argc, char** argv,
 	// initialization).
 	binpac::FlowBuffer::Policy flowbuffer_policy;
 	flowbuffer_policy.max_capacity = global_scope()->Lookup(
-		"BinPAC::flowbuffer_capacity_max")->ID_Val()->AsCount();
+		"BinPAC::flowbuffer_capacity_max")->GetVal()->AsCount();
 	flowbuffer_policy.min_capacity = global_scope()->Lookup(
-		"BinPAC::flowbuffer_capacity_min")->ID_Val()->AsCount();
+		"BinPAC::flowbuffer_capacity_min")->GetVal()->AsCount();
 	flowbuffer_policy.contract_threshold = global_scope()->Lookup(
-		"BinPAC::flowbuffer_contract_threshold")->ID_Val()->AsCount();
+		"BinPAC::flowbuffer_contract_threshold")->GetVal()->AsCount();
 	binpac::init(&flowbuffer_policy);
 
 	plugin_mgr->InitBifs();

src/zeek.bif

@@ -1926,7 +1926,7 @@ function global_sizes%(%): var_sizes
 		if ( id->HasVal() )
 			{
 			auto id_name = make_intrusive<StringVal>(id->Name());
-			auto id_size = val_mgr->Count(id->ID_Val()->MemoryAllocation());
+			auto id_size = val_mgr->Count(id->GetVal()->MemoryAllocation());
 			sizes->Assign(id_name.get(), std::move(id_size));
 			}
 		}
@@ -1959,11 +1959,7 @@ function global_ids%(%): id_table
 		rec->Assign(5, val_mgr->Bool(id->IsRedefinable()));
 
 		if ( id->HasVal() )
-			{
-			Val* val = id->ID_Val();
-			Ref(val);
-			rec->Assign(6, val);
-			}
+			rec->Assign(6, id->GetVal());
 
 		auto id_name = make_intrusive<StringVal>(id->Name());
 		ids->Assign(id_name.get(), std::move(rec));
@@ -1984,10 +1980,10 @@ function lookup_ID%(id: string%) : any
 	if ( ! i )
 		return make_intrusive<StringVal>("<unknown id>");
 
-	if ( ! i->ID_Val() )
+	if ( ! i->GetVal() )
 		return make_intrusive<StringVal>("<no ID value>");
 
-	return IntrusivePtr{NewRef{}, i->ID_Val()};
+	return i->GetVal();
 	%}
 
 ## Generates metadata about a record's fields. The returned information
@@ -3908,7 +3904,7 @@ static Val* mmdb_getvalue(MMDB_entry_data_s* entry_data, int status,
 static bool mmdb_try_open_loc ()
 	{
 	// City database is always preferred over Country database.
-	auto mmdb_dir_val = global_scope()->Lookup("mmdb_dir")->ID_Val();
+	const auto& mmdb_dir_val = global_scope()->Lookup("mmdb_dir")->GetVal();
 	std::string mmdb_dir = mmdb_dir_val->AsString()->CheckString();
 
 	if ( ! mmdb_dir.empty() )
@@ -3936,7 +3932,7 @@ static bool mmdb_try_open_loc ()
 
 static bool mmdb_try_open_asn ()
 	{
-	auto mmdb_dir_val = global_scope()->Lookup("mmdb_dir")->ID_Val();
+	const auto& mmdb_dir_val = global_scope()->Lookup("mmdb_dir")->GetVal();
 	std::string mmdb_dir = mmdb_dir_val->AsString()->CheckString();
 
 	if ( ! mmdb_dir.empty() )

src/zeekygen/IdentifierInfo.cc

@@ -16,8 +16,8 @@ IdentifierInfo::IdentifierInfo(IntrusivePtr<ID> arg_id, ScriptInfo* script)
 	  comments(), id(std::move(arg_id)), initial_val(), redefs(), fields(),
 	  last_field_seen(), declaring_script(script)
 	{
-	if ( id->ID_Val() && (id->IsOption() || id->IsRedefinable()) )
-		initial_val = id->ID_Val()->Clone();
+	if ( id->GetVal() && (id->IsOption() || id->IsRedefinable()) )
+		initial_val = id->GetVal()->Clone();
 	}
 
 IdentifierInfo::~IdentifierInfo()

src/zeekygen/IdentifierInfo.h

@@ -42,8 +42,8 @@ public:
 	/**
 	 * Returns the initial value of the identifier.
 	 */
-	Val* InitialVal() const
-		{ return initial_val.get(); }
+	const IntrusivePtr<Val>& InitialVal() const
+		{ return initial_val; }
 
 	/**
 	 * Add a comment associated with the identifier.  If the identifier is a