Merge remote-tracking branch 'origin/topic/awelzel/defer-more-stuff'

* origin/topic/awelzel/defer-more-stuff:
  RecordType: Ensure &default fields are always re-initialized
  Attr: Deprecate using &default and &optional together on record fields
  RecordType: Allow deferring &default=vector(), set(), table() fields

CHANGES

@@ -1,3 +1,17 @@
+8.0.0-dev.809 | 2025-07-30 10:28:06 +0200
+
+  * RecordType: Ensure &default fields are always re-initialized (Arne Welzel, Corelight)
+
+    This started working partly after the deferral logic introduced with
+    Zeek 6.0 so this finishes it :-)
+
+  * Attr: Deprecate using &default and &optional together on record fields (Arne Welzel, Corelight)
+
+    If &default implies re-initialization of the field, using them together
+    doesn't make much sense.
+
+  * RecordType: Allow deferring &default=vector(), set(), table() fields (Arne Welzel, Corelight)
+
 8.0.0-dev.805 | 2025-07-30 10:07:50 +0200
 
   * fixes for specialized ZAM operations needing to check whether record fields exist (Vern Paxson, Corelight)

NEWS

@@ -306,6 +306,10 @@ Changed Functionality
   `Analyzer::DebugLogging`. Furthermore the default options changed to enable
   more detailed output by default.
 
+- Record fields with a ``&default`` attribute are now consistently re-initialized
+  after deleting such fields. Previously, this would only work for constant
+  expressions, but has been extended to apply to arbitrary expressions.
+
 - Publishing remote events with vector arguments that contain holes is now
   rejected. The receiver side never had a chance to figure out where these
   holes would have been. There's a chance this breaks scripts that accidentally
@@ -339,6 +343,10 @@ Changed Functionality
 - The PPPoE parser now respects the size value given in the PPPoE header. Data
   beyon the size given in the header will be truncated.
 
+- Record fields with ``&default`` attributes initializing empty ``vector``, ``table``
+  or ``set`` instances are now deferred until they are accessed, potentially
+  improving memory usage when such fields are never accessed.
+
 Removed Functionality
 ---------------------
 
@@ -369,6 +377,10 @@ Deprecated Functionality
 
   The replacement script doesn't populate the ``email_body_sections`` anymore either.
 
+- Using ``&default`` and ``&optional`` together on a record field has been deprecated
+  as it would only result in ``&default`` behavior. This will become an error starting
+  with Zeek 8.1.
+
 - The ``zeek::Event()`` constructor was deprecated. Use ``event_mgr::Enqueue()``
   or ``event_mgr::Dispatch()`` instead.
 

VERSION

@@ -1 +1 @@
-8.0.0-dev.805
+8.0.0-dev.809

scripts/base/frameworks/notice/weird.zeek

@@ -52,7 +52,7 @@ export {
 		## The peer that originated this weird.  This is helpful in
 		## cluster deployments if a particular cluster node is having
 		## trouble to help identify which node is having trouble.
-		peer:   string  &log &optional &default=peer_description;
+		peer:   string  &log &default=peer_description;
 
 		## The source of the weird. When reported by an analyzer, this
 		## should be the name of the analyzer.

src/Attr.cc

@@ -5,6 +5,7 @@
 #include "zeek/Desc.h"
 #include "zeek/Expr.h"
 #include "zeek/IntrusivePtr.h"
+#include "zeek/Reporter.h"
 #include "zeek/Val.h"
 #include "zeek/input/Manager.h"
 #include "zeek/threading/SerialTypes.h"
@@ -298,6 +299,12 @@ bool Attributes::CheckAttr(Attr* a) {
         case ATTR_OPTIONAL:
             if ( global_var )
                 return AttrError("&optional is not valid for global variables");
+
+            // Remove in v8.1: Call AttrError()
+            if ( in_record && Find(ATTR_DEFAULT) )
+                zeek::reporter->Deprecation(
+                    "Remove in v8.1: Using &default and &optional together results in &default behavior");
+
             break;
 
         case ATTR_ADD_FUNC:
@@ -335,6 +342,11 @@ bool Attributes::CheckAttr(Attr* a) {
             if ( Find(ATTR_DEFAULT_INSERT) )
                 return AttrError("&default and &default_insert cannot be used together");
 
+            // Remove in v8.1: Call AttrError()
+            if ( in_record && Find(ATTR_OPTIONAL) )
+                zeek::reporter->Deprecation(
+                    "Remove in v8.1: Using &default and &optional together results in &default behavior");
+
             std::string err_msg;
             if ( ! check_default_attr(a, type, global_var, in_record, err_msg) && ! err_msg.empty() )
                 return AttrError(err_msg.c_str());

src/Type.cc

@@ -929,6 +929,20 @@ public:
             return rt->IsDeferrable() && constructor_list->Exprs().empty();
         }
 
+        // Allow deferring fairly common &default=vector() field initialization...
+        if ( init_expr->Tag() == EXPR_VECTOR_CONSTRUCTOR ) {
+            auto vce = zeek::cast_intrusive<VectorConstructorExpr>(init_expr);
+            return vce->GetType<zeek::VectorType>()->IsUnspecifiedVector();
+        }
+
+        // ...and also &default=table() and &default=set().
+        if ( init_expr->Tag() == EXPR_TABLE_COERCE || init_expr->Tag() == EXPR_TABLE_CONSTRUCTOR ||
+             init_expr->Tag() == EXPR_SET_CONSTRUCTOR ) {
+            auto une = zeek::cast_intrusive<UnaryExpr>(init_expr);
+            return une->Op()->GetType()->Tag() == TYPE_TABLE &&
+                   une->Op()->GetType<zeek::TableType>()->IsUnspecifiedTable();
+        }
+
         return false;
     }
 
@@ -1007,12 +1021,29 @@ private:
     void OptimizeCreationInits(RecordType* rt) {
         int i = 0;
         for ( auto& ci : rt->creation_inits ) {
-            if ( ! ci.second->IsDeferrable() )
+            if ( ! ci.second->IsDeferrable() ) {
                 rt->creation_inits[i++] = std::move(ci);
+
+                // A non-deferrable field with a &default attribute is expected to also exist in deferred_inits
+                // such that re-initialization after deletion of the field works.
+                if ( rt->FieldDecl(ci.first)->GetAttr(detail::ATTR_DEFAULT) != detail::Attr::nil ) {
+                    if ( ! rt->deferred_inits[ci.first] )
+                        zeek::reporter->InternalError("non-deferrable field %s$%s with &default not in deferred_inits",
+                                                      rt->GetName().c_str(), rt->FieldName(i));
+
+                    if ( rt->deferred_inits[ci.first] != rt->creation_inits[i - 1].second )
+                        zeek::reporter->InternalError("non-deferrable field %s$%s with &default has inconsistent inits",
+                                                      rt->GetName().c_str(), rt->FieldName(i));
+                }
+            }
             else {
-                // std::fprintf(stderr, "deferred %s$%s: %s\n", obj_desc_short(rt).c_str(), rt->FieldName(ci.first),
-                //             ci.second->InitExpr() ? obj_desc_short(ci.second->InitExpr()).c_str() : "<none>");
-                assert(! rt->deferred_inits[ci.first]);
+                // If deferred_inits already has a value, it should be the same as the one
+                // stored in creation_inits. This happens for deferrable record fields
+                // that have a &default attribute.
+                if ( rt->deferred_inits[ci.first] && rt->deferred_inits[ci.first] != ci.second )
+                    zeek::reporter->InternalError("deferrable field %s$%s has inconsistent inits",
+                                                  rt->GetName().c_str(), rt->FieldName(i));
+
                 rt->deferred_inits[ci.first].swap(ci.second);
             }
         }
@@ -1104,8 +1135,13 @@ void RecordType::AddField(unsigned int field, const TypeDecl* td) {
         }
 
         else {
-            auto efi = std::make_shared<detail::ExprFieldInit>(def_expr, type);
-            creation_inits.emplace_back(field, std::move(efi));
+            // Note that init is placed into creation_inits and deferred_inits
+            // such that accessing a record field after deleting it will run the
+            // &default expression to re-initialize it again.
+            //
+            // Also see RecordType::CreationInitsOptimizer.
+            init = std::make_shared<detail::ExprFieldInit>(def_expr, type);
+            creation_inits.emplace_back(field, init);
         }
     }
 

testing/btest/Baseline/language.delete-field-set-2/output

@@ -0,0 +1,4 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+default, [v=[1, 2, 3, 100], r=[c=101]]
+after changing, [v=[1, 2, 3, 100, 4711], r=[c=42]]
+after delete, [v=[1, 2, 3, 102], r=[c=103]]

testing/btest/Baseline/language.delete-field-set/output

@@ -1,2 +1,6 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-[a=<uninitialized>, b=<uninitialized>, c=<uninitialized>]
+[a={
+
+}, b={
+
+}, c=[]]

testing/btest/Baseline/language.optional-and-default-field/.stderr

@@ -0,0 +1,6 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+warning in <...>/optional-and-default-field.zeek, line 10: Remove in v8.1: Using &default and &optional together results in &default behavior
+warning in <...>/optional-and-default-field.zeek, line 11: Remove in v8.1: Using &default and &optional together results in &default behavior
+warning in <...>/optional-and-default-field.zeek, line 12: Remove in v8.1: Using &default and &optional together results in &default behavior
+warning in <...>/optional-and-default-field.zeek, line 13: Remove in v8.1: Using &default and &optional together results in &default behavior
+warning in <...>/optional-and-default-field.zeek, line 14: Remove in v8.1: Using &default and &optional together results in &default behavior

testing/btest/Baseline/language.optional-and-default-field/.stdout

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+[c=5, i=-5, v=[], r0=[], r1=[]]

testing/btest/language/delete-field-set.zeek

@@ -1,3 +1,5 @@
+# @TEST-DOC: Test behavior of &default when deleting a record field and subsequently accessing it again.
+#
 # @TEST-EXEC: zeek -b %INPUT >output 2>&1
 # @TEST-EXEC: btest-diff output
 
@@ -14,3 +16,30 @@ delete test$b;
 delete test$c;
 
 print test;
+
+# @TEST-START-NEXT
+global c = 99;
+
+# Helper function that's running as part of R's default construction.
+function seq(): count {
+	++c;
+	return c;
+}
+
+type R: record {
+	c: count &default=seq();
+};
+
+type FooBar: record {
+        v: vector of count &default=vector(1, 2, 3, seq());
+	r: R &default=R();
+};
+
+global test: FooBar;
+print "default", test;
+test$v += 4711;
+test$r$c = 42;
+print "after changing", test;
+delete test$v;
+delete test$r;
+print "after delete", test;

testing/btest/language/optional-and-default-field.zeek

@@ -0,0 +1,18 @@
+# @TEST-DOC: Warn on record fields that have both, &optional and &default
+#
+# @TEST-EXEC: zeek -b %INPUT
+# @TEST-EXEC: btest-diff .stdout
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
+
+type R: record { };
+
+type X: record {
+        c: count &optional &default=5;
+        i: int &default=-5 &optional;
+        v: vector of string &optional &default=vector();
+        r0: R &optional &default=R();
+        r1: R &default=R() &optional;
+};
+
+global x = X();
+print x;