Add &default_insert attribute for tables

This is based on the discussion in zeek/zeek#2668. Using &default with tables
can be confusing as the default value is not inserted. The following example
prints an empty table at the end even new Service records was instantiated.

    type Service: record {
        occurrences: count &default=0;
        last_seen: time &default=network_time();
    };

    global services: table[string] of Service &default=Service();

    event zeek_init()
        {
        services["http"]$occurrences += 1;
        services["http"]$last_seen = network_time();

        print services;
        }

Changing above &default to &default_insert will insert the newly created
default value upon a missed lookup and act less surprising.

Other examples that caused confusion previously revolved around table of sets
 or table of vectors and `add` or `+=` not working as expected.

    tbl_of_vector["http"] += 1
    add tbl_of_set["http"][1];

NEWS

@@ -37,6 +37,11 @@ New Functionality
   the break statement within ``assertion_failure()`` or ``assertion_result()``
   allows to suppress the default message.
 
+- Add a new ``&default_insert`` attribute for tables. This behaves as ``&default``
+  with the addition that the default value is inserted into the table upon a
+  failed lookup. Particularly for tables with nested container values, the
+  ``&default`` behavior of not inserting the value can be of little use.
+
 - The ``from_json()`` function now takes an optional key_func argument to
   normalize JSON object key names. This can be useful if the keys in a JSON
   object are not valid Zeek identifiers or reserved keywords.

src/Attr.cc

@@ -16,9 +16,12 @@ namespace zeek::detail
 
 const char* attr_name(AttrTag t)
 	{
+	// Do not collapse the list.
+	// clang-format off
 	static const char* attr_names[int(NUM_ATTRS)] = {
 		"&optional",
 		"&default",
+		"&default_insert",
 		"&redef",
 		"&add_func",
 		"&delete_func",
@@ -42,6 +45,7 @@ const char* attr_name(AttrTag t)
 		"&is_used",
 		"&ordered",
 	};
+	// clang-format on
 
 	return attr_names[int(t)];
 	}
@@ -359,8 +363,35 @@ void Attributes::CheckAttr(Attr* a)
 			}
 			break;
 
+		case ATTR_DEFAULT_INSERT:
+			{
+			if ( ! type->IsTable() )
+				{
+				Error("&default_insert only applicable to tables");
+				break;
+				}
+
+			if ( Find(ATTR_DEFAULT) )
+				{
+				Error("&default and &default_insert cannot be used together");
+				break;
+				}
+
+			std::string err_msg;
+			if ( ! check_default_attr(a, type, global_var, in_record, err_msg) &&
+			     ! err_msg.empty() )
+				Error(err_msg.c_str());
+			break;
+			}
+
 		case ATTR_DEFAULT:
 			{
+			if ( Find(ATTR_DEFAULT_INSERT) )
+				{
+				Error("&default and &default_insert cannot be used together");
+				break;
+				}
+
 			std::string err_msg;
 			if ( ! check_default_attr(a, type, global_var, in_record, err_msg) &&
 			     ! err_msg.empty() )
@@ -672,11 +703,13 @@ bool Attributes::operator==(const Attributes& other) const
 bool check_default_attr(Attr* a, const TypePtr& type, bool global_var, bool in_record,
                         std::string& err_msg)
 	{
+	ASSERT(a->Tag() == ATTR_DEFAULT || a->Tag() == ATTR_DEFAULT_INSERT);
+	std::string aname = attr_name(a->Tag());
 	// &default is allowed for global tables, since it's used in
 	// initialization of table fields. It's not allowed otherwise.
 	if ( global_var && ! type->IsTable() )
 		{
-		err_msg = "&default is not valid for global variables except for tables";
+		err_msg = aname + " is not valid for global variables except for tables";
 		return false;
 		}
 
@@ -707,7 +740,7 @@ bool check_default_attr(Attr* a, const TypePtr& type, bool global_var, bool in_r
 			return true;
 			}
 
-		a->GetExpr()->Error("&default value has inconsistent type", type.get());
+		a->GetExpr()->Error(util::fmt("%s value has inconsistent type", aname.c_str()), type.get());
 		return false;
 		}
 
@@ -725,7 +758,7 @@ bool check_default_attr(Attr* a, const TypePtr& type, bool global_var, bool in_r
 			FuncType* f = atype->AsFuncType();
 			if ( ! f->CheckArgs(tt->GetIndexTypes()) || ! same_type(f->Yield(), ytype) )
 				{
-				err_msg = "&default function type clash";
+				err_msg = aname + " function type clash";
 				return false;
 				}
 
@@ -748,7 +781,7 @@ bool check_default_attr(Attr* a, const TypePtr& type, bool global_var, bool in_r
 			return true;
 			}
 
-		err_msg = "&default value has inconsistent type";
+		err_msg = aname + " value has inconsistent type";
 		return false;
 		}
 

src/Attr.h

@@ -30,6 +30,7 @@ enum AttrTag
 	{
 	ATTR_OPTIONAL,
 	ATTR_DEFAULT,
+	ATTR_DEFAULT_INSERT, // insert default value on failed lookups
 	ATTR_REDEF,
 	ATTR_ADD_FUNC,
 	ATTR_DEL_FUNC,

src/Expr.cc

@@ -5364,7 +5364,17 @@ ExprPtr check_and_promote_expr(ExprPtr e, TypePtr t)
 			if ( e->Tag() == EXPR_TABLE_CONSTRUCTOR )
 				{
 				auto& attrs = cast_intrusive<TableConstructorExpr>(e)->GetAttrs();
-				auto& def = attrs ? attrs->Find(ATTR_DEFAULT) : nullptr;
+				zeek::detail::AttrPtr def = Attr::nil;
+
+				// Check for &default or &default_insert expressions
+				// and use it for type checking against t.
+				if ( attrs )
+					{
+					def = attrs->Find(ATTR_DEFAULT);
+					if ( ! def )
+						def = attrs->Find(ATTR_DEFAULT_INSERT);
+					}
+
 				if ( def )
 					{
 					std::string err_msg;

src/Val.cc

@@ -2137,7 +2137,10 @@ bool TableVal::IsSubsetOf(const TableVal& tv) const
 
 ValPtr TableVal::Default(const ValPtr& index)
 	{
-	const auto& def_attr = GetAttr(detail::ATTR_DEFAULT);
+	auto def_attr = GetAttr(detail::ATTR_DEFAULT);
+
+	if ( ! def_attr )
+		def_attr = GetAttr(detail::ATTR_DEFAULT_INSERT);
 
 	if ( ! def_attr )
 		return nullptr;
@@ -2268,7 +2271,13 @@ ValPtr TableVal::FindOrDefault(const ValPtr& index)
 	if ( auto rval = Find(index) )
 		return rval;
 
-	return Default(index);
+	// If the default came from a &default_insert attribute,
+	// insert the value upon a missed lookup.
+	auto def = Default(index);
+	if ( def && GetAttr(detail::ATTR_DEFAULT_INSERT) )
+		Assign(index, def);
+
+	return def;
 	}
 
 bool TableVal::Contains(const IPAddr& addr) const
@@ -2768,7 +2777,9 @@ void TableVal::InitDefaultFunc(detail::Frame* f)
 	if ( def_val )
 		return;
 
-	const auto& def_attr = GetAttr(detail::ATTR_DEFAULT);
+	auto def_attr = GetAttr(detail::ATTR_DEFAULT);
+	if ( ! def_attr )
+		def_attr = GetAttr(detail::ATTR_DEFAULT_INSERT);
 
 	if ( ! def_attr )
 		return;

src/Val.h

@@ -878,11 +878,13 @@ public:
 
 	/**
 	 * Finds an index in the table and returns its associated value or else
-	 * the &default value.
+	 * the &default or &default_insert value. If the &default_insert attribute
+	 * is set on the table, the returned value is also inserted into the table.
 	 * @param index  The index to lookup in the table.
 	 * @return  The value associated with the index.  If the index doesn't
-	 * exist, instead returns the &default value.  If there's no &default
-	 * attribute, then nullptr is still returned for nonexistent index.
+	 * exist, instead returns the &default or &default_insert.  If there's no
+	 * &default or &default_insert attribute, then nullptr is still returned
+	 * for nonexistent index.
 	 */
 	ValPtr FindOrDefault(const ValPtr& index);
 

src/parse.y

@@ -5,7 +5,7 @@
 // Switching parser table type fixes ambiguity problems.
 %define lr.type ielr
 
-%expect 211
+%expect 217
 
 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY TOK_ASSERT
 %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF
@@ -23,7 +23,7 @@
 %token TOK_WHILE TOK_AS TOK_IS
 %token TOK_GLOBAL_ID
 
-%token TOK_ATTR_ADD_FUNC TOK_ATTR_DEFAULT TOK_ATTR_OPTIONAL TOK_ATTR_REDEF
+%token TOK_ATTR_ADD_FUNC TOK_ATTR_DEFAULT TOK_ATTR_DEFAULT_INSERT TOK_ATTR_OPTIONAL TOK_ATTR_REDEF
 %token TOK_ATTR_DEL_FUNC TOK_ATTR_EXPIRE_FUNC
 %token TOK_ATTR_EXPIRE_CREATE TOK_ATTR_EXPIRE_READ TOK_ATTR_EXPIRE_WRITE
 %token TOK_ATTR_RAW_OUTPUT TOK_ATTR_ON_CHANGE TOK_ATTR_BROKER_STORE
@@ -1720,6 +1720,8 @@ attr_list:
 attr:
 		TOK_ATTR_DEFAULT '=' expr
 			{ $$ = new Attr(ATTR_DEFAULT, {AdoptRef{}, $3}); }
+	|	TOK_ATTR_DEFAULT_INSERT '=' expr
+			{ $$ = new Attr(ATTR_DEFAULT_INSERT, {AdoptRef{}, $3}); }
 	|	TOK_ATTR_OPTIONAL
 			{ $$ = new Attr(ATTR_OPTIONAL); }
 	|	TOK_ATTR_REDEF

src/scan.l

@@ -377,6 +377,7 @@ when	return TOK_WHEN;
 &add_func	return TOK_ATTR_ADD_FUNC;
 &create_expire	return TOK_ATTR_EXPIRE_CREATE;
 &default	return TOK_ATTR_DEFAULT;
+&default_insert	return TOK_ATTR_DEFAULT_INSERT;
 &delete_func	return TOK_ATTR_DEL_FUNC;
 &deprecated	return TOK_ATTR_DEPRECATED;
 &raw_output	return TOK_ATTR_RAW_OUTPUT;

testing/btest/Baseline/language.table-default-insert-copy/.stderr

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/Baseline/language.table-default-insert-copy/out

@@ -0,0 +1,10 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+copy_tbl[0], no-default
+copy_tbl[1], <default>
+copy_tbl, {
+[0] = no-default,
+[1] = <default>
+}
+tbl, {
+[0] = no-default
+}

testing/btest/Baseline/language.table-default-insert-errors-2/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/table-default-insert-errors.zeek, line 2: &default_insert only applicable to tables (&default_insert=a)

testing/btest/Baseline/language.table-default-insert-errors-3/.stderr

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in string and <...>/table-default-insert-errors.zeek, line 2: arithmetic mixed with non-arithmetic (string and 1)
+error in <...>/table-default-insert-errors.zeek, line 2: &default_insert value has inconsistent type (&default_insert=1)

testing/btest/Baseline/language.table-default-insert-errors-4/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/table-default-insert-errors.zeek, line 3: &default_insert function type clash (&default_insert=function(){ return (c)})

testing/btest/Baseline/language.table-default-insert-errors-5/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/table-default-insert-errors.zeek, line 3: &default_insert function type clash (&default_insert=function(){ return (c)})

testing/btest/Baseline/language.table-default-insert-errors-6/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/table-default-insert-errors.zeek, line 3: &default and &default_insert cannot be used together (&default=a, &default_insert=b)

testing/btest/Baseline/language.table-default-insert-errors-7/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/table-default-insert-errors.zeek, line 2: &default and &default_insert cannot be used together (&default_insert=a, &default=b)

testing/btest/Baseline/language.table-default-insert-errors/.stderr

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+error in <...>/table-default-insert-errors.zeek, line 8: &default_insert only applicable to tables (&default_insert=a)

testing/btest/Baseline/language.table-default-insert/.stderr

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/Baseline/language.table-default-insert/out

@@ -0,0 +1,28 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+table[count] of R
+===
+<default>
+{
+[0] = <default>
+}
+===
+[a, b]
+[a, b]
+{
+[0] = [a, b, c],
+[1] = [a, b, d]
+}
+===
+[a=0]
+[a=1]
+{
+[0] = [a=0],
+[1] = [a=1]
+}
+===
+[a=0]
+[a=1]
+{
+[0] = [a=0],
+[1] = [a=1]
+}

testing/btest/language/table-default-insert-copy.zeek

@@ -0,0 +1,21 @@
+# @TEST-DOC: Ensure &default_insert is copied with a table.
+# @TEST-EXEC: zeek -b %INPUT >out
+# @TEST-EXEC: btest-diff out
+# @TEST-EXEC: btest-diff .stderr
+
+global tbl: table[count] of string &default_insert="<default>";
+
+event zeek_init()
+	{
+	tbl[0] = "no-default";
+	local copy_tbl = copy(tbl);
+	print "copy_tbl[0]", copy_tbl[0];
+	print "copy_tbl[1]", copy_tbl[1];
+	print "copy_tbl", copy_tbl;
+	print "tbl", tbl;
+
+	assert 0 in copy_tbl;
+	assert 1 in copy_tbl;
+	assert |copy_tbl| == 2;
+	assert |tbl| == 1;
+	}

testing/btest/language/table-default-insert-errors.zeek

@@ -0,0 +1,36 @@
+# @TEST-DOC: Bad &default_insert usage.
+#
+# @TEST-EXEC-FAIL: zeek -b %INPUT
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+# Not applicable to record fields.
+type R: record {
+	a: string &default_insert="a";
+};
+
+@TEST-START-NEXT
+# Not applicable to sets.
+global s: set[string] &default_insert="a";
+
+@TEST-START-NEXT
+# Wrong expression type
+global tbl: table[count] of string &default_insert=1;
+
+@TEST-START-NEXT
+
+# default function has wrong type
+global tbl: table[count] of string &default_insert=function(c: count): count { return c; };
+
+@TEST-START-NEXT
+
+# default function has wrong type for inferred type
+global tbl = table([1] = "a") &default_insert=function(c: count): count { return c; };
+
+@TEST-START-NEXT
+
+# Using &default and &default_insert together does not work.
+global tbl: table[count] of string &default="a" &default_insert="b";
+
+@TEST-START-NEXT
+# Using &default and &default_insert together does not work, reversed order.
+global tbl: table[count] of string &default_insert="a" &default="b";

testing/btest/language/table-default-insert.zeek

@@ -0,0 +1,40 @@
+# @TEST-EXEC: zeek -b %INPUT >out
+# @TEST-EXEC: btest-diff out
+# @TEST-EXEC: btest-diff .stderr
+
+global tbl: table[count] of string &default_insert="<default>";
+global tbl_vec: table[count] of vector of string &default_insert=vector("a", "b");
+
+type R: record {
+	a: string;
+};
+global tbl_def_func: table[count] of R &default_insert=function(c: count): R { return R($a=cat(c)); };
+
+# This takes a different code path than without a table constructor.
+global tbl_construct = table([1] = R($a="1")) &default_insert=function(c: count): R { return R($a=cat(c)); };
+
+event zeek_init()
+	{
+	print type_name(tbl_construct);
+
+	print "===";
+	print tbl[0];
+	print tbl;
+
+	print "===";
+	print tbl_vec[0];
+	print tbl_vec[1];
+	tbl_vec[0] += "c";
+	tbl_vec[1] += "d";
+	print tbl_vec;
+
+	print "===";
+	print tbl_def_func[0];
+	print tbl_def_func[1];
+	print tbl_def_func;
+
+	print "===";
+	print tbl_construct[0];
+	print tbl_construct[1];
+	print tbl_construct;
+	}