RecordType: Ensure &default fields are always re-initialized

This started working partly after the deferral logic introduced with
Zeek 6.0 so this finishes it :-)

NEWS

@@ -298,6 +298,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

src/Type.cc

@@ -1021,14 +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 (%s)\n", obj_desc_short(rt).c_str(),
+                // If deferred_inits already has a value, it should be the same as the one
-                // rt->FieldName(ci.first),
+                // stored in creation_inits. This happens for deferrable record fields
-                //             ci.second->InitExpr() ? obj_desc_short(ci.second->InitExpr()).c_str() : "<none>",
+                // that have a &default attribute.
-                //            typeid(ci).name());
+                if ( rt->deferred_inits[ci.first] && rt->deferred_inits[ci.first] != ci.second )
-                assert(! rt->deferred_inits[ci.first]);
+                    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);
             }
         }
@@ -1120,8 +1135,13 @@ void RecordType::AddField(unsigned int field, const TypeDecl* td) {
         }
 
         else {
-            auto efi = std::make_shared<detail::ExprFieldInit>(def_expr, type);
+            // Note that init is placed into creation_inits and deferred_inits
-            creation_inits.emplace_back(field, std::move(efi));
+            // 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/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;