speeding up record constructors

src/Expr.cc

@@ -94,11 +94,11 @@ const char* expr_name(ExprTag t) {
         "$=",
         "$=$",
         "$+=$",
+        "[=+$]",
         "from_any_vec_coerce",
         "any[]",
         "ZAM-builtin()",
         "nop",
-
     };
 
     if ( int(t) >= NUM_EXPRS ) {
@@ -2870,7 +2870,8 @@ RecordConstructorExpr::RecordConstructorExpr(ListExprPtr constructor_list)
         Error("bad type in record constructor", constructor_error_expr);
 }
 
-RecordConstructorExpr::RecordConstructorExpr(RecordTypePtr known_rt, ListExprPtr constructor_list)
+RecordConstructorExpr::RecordConstructorExpr(RecordTypePtr known_rt, ListExprPtr constructor_list,
+                                             bool no_mandatory_fields)
     : Expr(EXPR_RECORD_CONSTRUCTOR), op(std::move(constructor_list)) {
     if ( IsError() )
         return;
@@ -2911,6 +2912,9 @@ RecordConstructorExpr::RecordConstructorExpr(RecordTypePtr known_rt, ListExprPtr
     if ( IsError() )
         return;
 
+    if ( no_mandatory_fields )
+        return;
+
     auto n = known_rt->NumFields();
     for ( i = 0; i < n; ++i )
         if ( fields_seen.count(i) == 0 ) {

src/Expr.h

@@ -103,6 +103,7 @@ enum ExprTag : int {
     EXPR_FIELD_LHS_ASSIGN,
     EXPR_REC_ASSIGN_FIELDS,
     EXPR_REC_ADD_FIELDS,
+    EXPR_REC_CONSTRUCT_WITH_REC,
     EXPR_FROM_ANY_VEC_COERCE,
     EXPR_ANY_INDEX,
     EXPR_SCRIPT_OPT_BUILTIN,
@@ -1126,7 +1127,10 @@ public:
     explicit RecordConstructorExpr(ListExprPtr constructor_list);
 
     // This form is used to construct records of a known (ultimate) type.
-    explicit RecordConstructorExpr(RecordTypePtr known_rt, ListExprPtr constructor_list);
+    // The flag allows skipping of checking for mandatory fields, for
+    // script optimization that may elide them.
+    explicit RecordConstructorExpr(RecordTypePtr known_rt, ListExprPtr constructor_list,
+                                   bool no_mandatory_fields = false);
 
     ListExprPtr Op() const { return op; }
     const auto& Map() const { return map; }

src/script_opt/CSE.cc

@@ -140,6 +140,7 @@ TraversalCode CSE_ValidityChecker::PreExpr(const Expr* e) {
 
         case EXPR_RECORD_COERCE:
         case EXPR_RECORD_CONSTRUCTOR:
+        case EXPR_REC_CONSTRUCT_WITH_REC:
             // Note, record coercion behaves like constructors in terms of
             // potentially executing &default functions. In either case,
             // the type of the expression reflects the type we want to analyze

src/script_opt/Expr.cc

@@ -1768,7 +1768,7 @@ ExprPtr RecordConstructorExpr::Duplicate() {
 
     if ( map ) {
         auto rt = cast_intrusive<RecordType>(type);
-        return SetSucc(new RecordConstructorExpr(rt, op_l));
+        return SetSucc(new RecordConstructorExpr(rt, op_l, true));
     }
     else
         return SetSucc(new RecordConstructorExpr(op_l));
@@ -1792,6 +1792,12 @@ bool RecordConstructorExpr::HasReducedOps(Reducer* c) const {
 }
 
 ExprPtr RecordConstructorExpr::Reduce(Reducer* c, StmtPtr& red_stmt) {
+    static bool skip_chains = getenv("ZAM_SKIP_CHAINS");
+    if ( ConstructFromRecordExpr::FindMostCommonRecordSource(op) && ! skip_chains ) {
+        auto cfr = with_location_of(make_intrusive<ConstructFromRecordExpr>(this), this);
+        return cfr->Reduce(c, red_stmt);
+    }
+
     red_stmt = ReduceToSingletons(c);
 
     if ( c->Optimizing() )
@@ -1801,28 +1807,6 @@ ExprPtr RecordConstructorExpr::Reduce(Reducer* c, StmtPtr& red_stmt) {
 }
 
 StmtPtr RecordConstructorExpr::ReduceToSingletons(Reducer* c) {
-    auto& exprs2 = op->AsListExpr()->Exprs();
-    int nfield = 0;
-    std::set<const ID*> field_names;
-    loop_over_list(exprs2, j) {
-        auto e_i = exprs2[j];
-        auto fa_i = e_i->AsFieldAssignExprPtr();
-        auto fa_i_rhs = e_i->GetOp1();
-
-        if ( fa_i_rhs->Tag() == EXPR_FIELD ) {
-            ++nfield;
-            auto op1 = fa_i_rhs->GetOp1();
-            if ( op1->Tag() == EXPR_NAME )
-                field_names.insert(op1->AsNameExpr()->Id());
-        }
-    }
-
-#if 0
-    if ( nfield > 0 )
-	printf("constructor with %d fields spanning %lu names: %s\n",
-		nfield, field_names.size(), obj_desc(this).c_str());
-#endif
-
     StmtPtr red_stmt;
     auto& exprs = op->AsListExpr()->Exprs();
 
@@ -2855,7 +2839,7 @@ static NameExprPtr get_RFU_RHS_var(const Stmt* s) {
     return cast_intrusive<NameExpr>(var);
 }
 
-RecordFieldUpdates::RecordFieldUpdates(ExprTag t, const std::vector<const Stmt*>& stmts,
+RecordFieldUpdatesExpr::RecordFieldUpdatesExpr(ExprTag t, const std::vector<const Stmt*>& stmts,
                                                std::set<const Stmt*>& stmt_pool)
     : BinaryExpr(t, get_RFU_LHS_var(stmts[0]), get_RFU_RHS_var(stmts[0])) {
     for ( auto s : stmts ) {
@@ -2877,14 +2861,14 @@ RecordFieldUpdates::RecordFieldUpdates(ExprTag t, const std::vector<const Stmt*>
     }
 }
 
-RecordFieldUpdates::RecordFieldUpdates(ExprTag t, ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map,
+RecordFieldUpdatesExpr::RecordFieldUpdatesExpr(ExprTag t, ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map,
                                                std::vector<int> _rhs_map)
     : BinaryExpr(t, std::move(e1), std::move(e2)) {
     lhs_map = std::move(_lhs_map);
     rhs_map = std::move(_rhs_map);
 }
 
-ValPtr RecordFieldUpdates::Fold(Val* v1, Val* v2) const {
+ValPtr RecordFieldUpdatesExpr::Fold(Val* v1, Val* v2) const {
     auto rv1 = v1->AsRecordVal();
     auto rv2 = v2->AsRecordVal();
 
@@ -2894,15 +2878,15 @@ ValPtr RecordFieldUpdates::Fold(Val* v1, Val* v2) const {
     return nullptr;
 }
 
-bool RecordFieldUpdates::IsReduced(Reducer* c) const { return HasReducedOps(c); }
+bool RecordFieldUpdatesExpr::IsReduced(Reducer* c) const { return HasReducedOps(c); }
 
-void RecordFieldUpdates::ExprDescribe(ODesc* d) const {
+void RecordFieldUpdatesExpr::ExprDescribe(ODesc* d) const {
     op1->Describe(d);
     d->Add(expr_name(tag));
     op2->Describe(d);
 }
 
-ExprPtr RecordFieldUpdates::Reduce(Reducer* c, StmtPtr& red_stmt) {
+ExprPtr RecordFieldUpdatesExpr::Reduce(Reducer* c, StmtPtr& red_stmt) {
     if ( c->Optimizing() ) {
         op1 = c->UpdateExpr(op1);
         op2 = c->UpdateExpr(op2);
@@ -2922,23 +2906,133 @@ ExprPtr RecordFieldUpdates::Reduce(Reducer* c, StmtPtr& red_stmt) {
     return ThisPtr();
 }
 
-ExprPtr AssignRecordFields::Duplicate() {
+ExprPtr AssignRecordFieldsExpr::Duplicate() {
     auto e1 = op1->Duplicate();
     auto e2 = op2->Duplicate();
-    return SetSucc(new AssignRecordFields(e1, e2, lhs_map, rhs_map));
+    return SetSucc(new AssignRecordFieldsExpr(e1, e2, lhs_map, rhs_map));
 }
 
-void AssignRecordFields::FoldField(RecordVal* rv1, RecordVal* rv2, size_t i) const {
+void AssignRecordFieldsExpr::FoldField(RecordVal* rv1, RecordVal* rv2, size_t i) const {
     rv1->Assign(lhs_map[i], rv2->GetField(rhs_map[i]));
 }
 
-ExprPtr AddRecordFields::Duplicate() {
+ConstructFromRecordExpr::ConstructFromRecordExpr(const RecordConstructorExpr* orig)
-    auto e1 = op1->Duplicate();
+    : AssignRecordFieldsExpr(nullptr, nullptr, {}, {}) {
-    auto e2 = op2->Duplicate();
+    tag = EXPR_REC_CONSTRUCT_WITH_REC;
-    return SetSucc(new AddRecordFields(e1, e2, lhs_map, rhs_map));
+    SetType(orig->GetType());
+
+    // Arguments used in original and final constructor.
+    auto& orig_args = orig->Op()->Exprs();
+    auto args = with_location_of(make_intrusive<ListExpr>(), orig); // we'll build this up
+
+    auto src_id = FindMostCommonRecordSource(orig->Op());
+    auto& map = orig->Map();
+
+    for ( size_t i = 0; i < orig_args.size(); ++i ) {
+        auto e = orig_args[i];
+        auto src = FindRecordSource(e);
+        if ( src ) {
+            auto id = src->GetOp1()->AsNameExpr()->IdPtr();
+            if ( id == src_id ) {
+                lhs_map.push_back(map ? (*map)[i] : i);
+                rhs_map.push_back(src->Field());
+            }
+        }
+        else
+            args->Append({NewRef{}, e});
     }
 
-void AddRecordFields::FoldField(RecordVal* rv1, RecordVal* rv2, size_t i) const {
+    auto rt = cast_intrusive<RecordType>(orig->GetType());
+    op1 = with_location_of(make_intrusive<RecordConstructorExpr>(std::move(rt), std::move(args), true), orig);
+    op2 = with_location_of(make_intrusive<NameExpr>(std::move(src_id)), orig);
+}
+
+IDPtr ConstructFromRecordExpr::FindMostCommonRecordSource(const ListExprPtr& exprs) {
+    // Maps identifiers to how often they appear in the constructor's
+    // arguments as a field reference.
+    std::unordered_map<IDPtr, int> id_cnt;
+
+    for ( auto e : exprs->Exprs() ) {
+        auto src = FindRecordSource(e);
+        if ( src ) {
+            auto id = src->GetOp1()->AsNameExpr()->IdPtr();
+            auto ic = id_cnt.find(id);
+            if ( ic == id_cnt.end() )
+                id_cnt[id] = 1;
+            else
+                ++ic->second;
+        }
+    }
+
+    size_t max = 0;
+    IDPtr max_ID;
+
+    for ( auto& cnt : id_cnt )
+        if ( cnt.second > max ) {
+            max = cnt.second;
+            max_ID = cnt.first;
+        }
+
+    return max_ID;
+}
+
+FieldExprPtr ConstructFromRecordExpr::FindRecordSource(const Expr* const_e) {
+    // The following just saves us from having to define a "const" version
+    // of AsFieldAssignExprPtr().
+    auto e = const_cast<Expr*>(const_e);
+    const auto fa = e->AsFieldAssignExprPtr();
+    auto fa_rhs = e->GetOp1();
+
+    if ( fa_rhs->Tag() != EXPR_FIELD )
+        return nullptr;
+
+    auto rhs_rec = fa_rhs->GetOp1();
+    if ( rhs_rec->Tag() != EXPR_NAME )
+        return nullptr;
+
+    return cast_intrusive<FieldExpr>(fa_rhs);
+}
+
+ExprPtr ConstructFromRecordExpr::Duplicate() {
+    auto e1 = op1->Duplicate();
+    auto e2 = op2->Duplicate();
+    return SetSucc(new ConstructFromRecordExpr(e1, e2, lhs_map, rhs_map));
+}
+
+bool ConstructFromRecordExpr::IsReduced(Reducer* c) const { return op1->HasReducedOps(c) && op2->IsReduced(c); }
+
+bool ConstructFromRecordExpr::HasReducedOps(Reducer* c) const { return IsReduced(c); }
+
+ExprPtr ConstructFromRecordExpr::Reduce(Reducer* c, StmtPtr& red_stmt) {
+    if ( c->Optimizing() ) {
+        op1 = c->UpdateExpr(op1);
+        op2 = c->UpdateExpr(op2);
+    }
+
+    red_stmt = nullptr;
+
+    if ( ! op1->HasReducedOps(c) )
+        red_stmt = op1->ReduceToSingletons(c);
+
+    StmtPtr red2_stmt;
+    if ( ! op2->IsSingleton(c) )
+        op2 = op2->ReduceToSingleton(c, red2_stmt);
+
+    red_stmt = MergeStmts(red_stmt, std::move(red2_stmt));
+
+    if ( c->Optimizing() )
+        return ThisPtr();
+    else
+        return AssignToTemporary(c, red_stmt);
+}
+
+ExprPtr AddRecordFieldsExpr::Duplicate() {
+    auto e1 = op1->Duplicate();
+    auto e2 = op2->Duplicate();
+    return SetSucc(new AddRecordFieldsExpr(e1, e2, lhs_map, rhs_map));
+}
+
+void AddRecordFieldsExpr::FoldField(RecordVal* rv1, RecordVal* rv2, size_t i) const {
     // The goal here is correctness, not efficiency, since normally this
     // expression only exists temporarily before being compiled to ZAM.
     auto lhs_val = rv1->GetField(lhs_map[i]);

src/script_opt/Expr.h

@@ -106,7 +106,7 @@ protected:
 
 // Base class for updating a number of record fields from fields in
 // another record.
-class RecordFieldUpdates : public BinaryExpr {
+class RecordFieldUpdatesExpr : public BinaryExpr {
 public:
     const auto& LHSMap() const { return lhs_map; }
     const auto& RHSMap() const { return rhs_map; }
@@ -118,8 +118,8 @@ public:
     ExprPtr Reduce(Reducer* c, StmtPtr& red_stmt) override;
 
 protected:
-    RecordFieldUpdates(ExprTag t, const std::vector<const Stmt*>& stmts, std::set<const Stmt*>& stmt_pool);
+    RecordFieldUpdatesExpr(ExprTag t, const std::vector<const Stmt*>& stmts, std::set<const Stmt*>& stmt_pool);
-    RecordFieldUpdates(ExprTag t, ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map, std::vector<int> _rhs_map);
+    RecordFieldUpdatesExpr(ExprTag t, ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map, std::vector<int> _rhs_map);
 
     virtual void FoldField(RecordVal* rv1, RecordVal* rv2, size_t i) const = 0;
 
@@ -129,30 +129,59 @@ protected:
     std::vector<int> rhs_map;
 };
 
-class AssignRecordFields : public RecordFieldUpdates {
+// Assign a bunch of record fields en masse from fields in another record.
+class AssignRecordFieldsExpr : public RecordFieldUpdatesExpr {
 public:
-    AssignRecordFields(const std::vector<const Stmt*>& stmts, std::set<const Stmt*>& stmt_pool)
+    AssignRecordFieldsExpr(const std::vector<const Stmt*>& stmts, std::set<const Stmt*>& stmt_pool)
-        : RecordFieldUpdates(EXPR_REC_ASSIGN_FIELDS, stmts, stmt_pool) {}
+        : RecordFieldUpdatesExpr(EXPR_REC_ASSIGN_FIELDS, stmts, stmt_pool) {}
 
     ExprPtr Duplicate() override;
 
 protected:
-    AssignRecordFields(ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map, std::vector<int> _rhs_map)
+    // Used for duplicating.
-        : RecordFieldUpdates(EXPR_REC_ASSIGN_FIELDS, e1, e2, _lhs_map, _rhs_map) {}
+    AssignRecordFieldsExpr(ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map, std::vector<int> _rhs_map)
+        : RecordFieldUpdatesExpr(EXPR_REC_ASSIGN_FIELDS, e1, e2, _lhs_map, _rhs_map) {}
 
     void FoldField(RecordVal* rv1, RecordVal* rv2, size_t i) const override;
 };
 
-class AddRecordFields : public RecordFieldUpdates {
+// Construct a record with some of the fields taken directly from another
+// record. First operand is the base constructor (a subset of the original)
+// and the second is the source record being used for some of the
+// initialization.
+using FieldExprPtr = IntrusivePtr<FieldExpr>;
+class ConstructFromRecordExpr : public AssignRecordFieldsExpr {
 public:
-    AddRecordFields(const std::vector<const Stmt*>& stmts, std::set<const Stmt*>& stmt_pool)
+    ConstructFromRecordExpr(const RecordConstructorExpr* orig);
-        : RecordFieldUpdates(EXPR_REC_ADD_FIELDS, stmts, stmt_pool) {}
+
+    static IDPtr FindMostCommonRecordSource(const ListExprPtr& exprs);
+
+    ExprPtr Duplicate() override;
+
+    bool IsReduced(Reducer* c) const override;
+    bool HasReducedOps(Reducer* c) const override;
+    ExprPtr Reduce(Reducer* c, StmtPtr& red_stmt) override;
+
+protected:
+    ConstructFromRecordExpr(ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map, std::vector<int> _rhs_map)
+        : AssignRecordFieldsExpr(e1, e2, _lhs_map, _rhs_map) {
+        tag = EXPR_REC_CONSTRUCT_WITH_REC;
+    }
+
+    static FieldExprPtr FindRecordSource(const Expr* e);
+};
+
+// Add en masse fields from one record to fields in another record.
+class AddRecordFieldsExpr : public RecordFieldUpdatesExpr {
+public:
+    AddRecordFieldsExpr(const std::vector<const Stmt*>& stmts, std::set<const Stmt*>& stmt_pool)
+        : RecordFieldUpdatesExpr(EXPR_REC_ADD_FIELDS, stmts, stmt_pool) {}
 
     ExprPtr Duplicate() override;
 
 protected:
-    AddRecordFields(ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map, std::vector<int> _rhs_map)
+    AddRecordFieldsExpr(ExprPtr e1, ExprPtr e2, std::vector<int> _lhs_map, std::vector<int> _rhs_map)
-        : RecordFieldUpdates(EXPR_REC_ADD_FIELDS, e1, e2, _lhs_map, _rhs_map) {}
+        : RecordFieldUpdatesExpr(EXPR_REC_ADD_FIELDS, e1, e2, _lhs_map, _rhs_map) {}
 
     void FoldField(RecordVal* rv1, RecordVal* rv2, size_t i) const override;
 };

src/script_opt/ProfileFunc.cc

@@ -444,7 +444,8 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e) {
             return TC_ABORTSTMT;
         }
 
-        case EXPR_RECORD_CONSTRUCTOR: CheckRecordConstructor(e->GetType()); break;
+        case EXPR_RECORD_CONSTRUCTOR:
+        case EXPR_REC_CONSTRUCT_WITH_REC: CheckRecordConstructor(e->GetType()); break;
 
         case EXPR_SET_CONSTRUCTOR: {
             auto sc = static_cast<const SetConstructorExpr*>(e);

src/script_opt/Reduce.cc

@@ -125,6 +125,7 @@ static bool same_expr(const Expr* e1, const Expr* e2, bool check_defs) {
 
         case EXPR_CLONE:
         case EXPR_RECORD_CONSTRUCTOR:
+        case EXPR_REC_CONSTRUCT_WITH_REC:
         case EXPR_TABLE_CONSTRUCTOR:
         case EXPR_SET_CONSTRUCTOR:
         case EXPR_VECTOR_CONSTRUCTOR:
@@ -490,7 +491,8 @@ bool Reducer::ExprValid(const ID* id, const Expr* e1, const Expr* e2) const {
                 has_side_effects = true;
         }
 
-        else if ( e1->Tag() == EXPR_RECORD_CONSTRUCTOR || e1->Tag() == EXPR_RECORD_COERCE )
+        else if ( e1->Tag() == EXPR_RECORD_CONSTRUCTOR || e1->Tag() == EXPR_REC_CONSTRUCT_WITH_REC ||
+                  e1->Tag() == EXPR_RECORD_COERCE )
             has_side_effects = pfs->HasSideEffects(SideEffectsOp::CONSTRUCTION, e1->GetType());
 
         e1_se = ExprSideEffects(has_side_effects);

src/script_opt/ScriptOpt.cc

@@ -592,7 +592,7 @@ void analyze_scripts(bool no_unused_warnings) {
             func.SetShouldNotAnalyze();
 
     if ( ! have_one_to_do )
-        reporter->FatalError("no matching functions/files for C++ compilation");
+        reporter->FatalError("no matching functions/files for script optimization");
 
     if ( CPP_init_hook ) {
         (*CPP_init_hook)();

src/script_opt/Stmt.cc

@@ -887,9 +887,9 @@ StmtPtr StmtList::TransformChain(const OpChain& c, ExprTag t, std::set<const Stm
 
         ExprPtr e;
         if ( t == EXPR_ASSIGN )
-            e = make_intrusive<AssignRecordFields>(orig_s, chain_stmts);
+            e = make_intrusive<AssignRecordFieldsExpr>(orig_s, chain_stmts);
         else
-            e = make_intrusive<AddRecordFields>(orig_s, chain_stmts);
+            e = make_intrusive<AddRecordFieldsExpr>(orig_s, chain_stmts);
 
         e->SetLocationInfo(sl->GetLocationInfo());
         auto es = with_location_of(make_intrusive<ExprStmt>(std::move(e)), sl);

src/script_opt/UseDefs.cc

@@ -5,6 +5,7 @@
 #include "zeek/Desc.h"
 #include "zeek/Reporter.h"
 #include "zeek/Stmt.h"
+#include "zeek/script_opt/Expr.h"
 #include "zeek/script_opt/Reduce.h"
 #include "zeek/script_opt/ScriptOpt.h"
 
@@ -453,6 +454,14 @@ UDs UseDefs::ExprUDs(const Expr* e) {
             break;
         }
 
+        case EXPR_REC_CONSTRUCT_WITH_REC: {
+            auto rcw = static_cast<const RecordFieldUpdatesExpr*>(e);
+            auto constructor_UDs = ExprUDs(e->GetOp1().get());
+            AddInExprUDs(uds, e->GetOp2().get());
+            uds = UD_Union(uds, constructor_UDs);
+            break;
+        }
+
         case EXPR_TABLE_CONSTRUCTOR: {
             auto t = static_cast<const TableConstructorExpr*>(e);
             AddInExprUDs(uds, t->GetOp1().get());

src/script_opt/ZAM/Compile.h

@@ -189,7 +189,7 @@ private:
     const ZAMStmt CompileAddToExpr(const AddToExpr* e);
     const ZAMStmt CompileRemoveFromExpr(const RemoveFromExpr* e);
     const ZAMStmt CompileAssignExpr(const AssignExpr* e);
-    const ZAMStmt CompileRecFieldUpdates(const RecordFieldUpdates* e);
+    const ZAMStmt CompileRecFieldUpdates(const RecordFieldUpdatesExpr* e);
     const ZAMStmt CompileZAMBuiltin(const NameExpr* lhs, const ScriptOptBuiltinExpr* zbi);
     const ZAMStmt CompileAssignToIndex(const NameExpr* lhs, const IndexExpr* rhs);
     const ZAMStmt CompileFieldLHSAssignExpr(const FieldLHSAssignExpr* e);
@@ -245,7 +245,9 @@ private:
 
     const ZAMStmt ConstructTable(const NameExpr* n, const Expr* e);
     const ZAMStmt ConstructSet(const NameExpr* n, const Expr* e);
-    const ZAMStmt ConstructRecord(const NameExpr* n, const Expr* e);
+    const ZAMStmt ConstructRecord(const NameExpr* n, const Expr* e) { return ConstructRecord(n, e, false); }
+    const ZAMStmt ConstructRecordFromRecord(const NameExpr* n, const Expr* e) { return ConstructRecord(n, e, true); }
+    const ZAMStmt ConstructRecord(const NameExpr* n, const Expr* e, bool is_from_rec);
     const ZAMStmt ConstructVector(const NameExpr* n, const Expr* e);
 
     const ZAMStmt ArithCoerce(const NameExpr* n, const Expr* e);

src/script_opt/ZAM/Expr.cc

@@ -23,7 +23,7 @@ const ZAMStmt ZAMCompiler::CompileExpr(const Expr* e) {
         case EXPR_ASSIGN: return CompileAssignExpr(static_cast<const AssignExpr*>(e));
 
         case EXPR_REC_ASSIGN_FIELDS:
-        case EXPR_REC_ADD_FIELDS: return CompileRecFieldUpdates(static_cast<const RecordFieldUpdates*>(e));
+        case EXPR_REC_ADD_FIELDS: return CompileRecFieldUpdates(static_cast<const RecordFieldUpdatesExpr*>(e));
 
         case EXPR_INDEX_ASSIGN: {
             auto iae = static_cast<const IndexAssignExpr*>(e);
@@ -230,15 +230,14 @@ const ZAMStmt ZAMCompiler::CompileAssignExpr(const AssignExpr* e) {
 #include "ZAM-GenExprsDefsV.h"
 }
 
-const ZAMStmt ZAMCompiler::CompileRecFieldUpdates(const RecordFieldUpdates* e) {
+const ZAMStmt ZAMCompiler::CompileRecFieldUpdates(const RecordFieldUpdatesExpr* e) {
-    auto lhs = e->GetOp1()->AsNameExpr();
     auto rhs = e->GetOp2()->AsNameExpr();
 
     auto& rhs_map = e->RHSMap();
 
     auto aux = new ZInstAux(0);
     aux->map = e->LHSMap();
-    aux->rhs_map = e->RHSMap();
+    aux->rhs_map = rhs_map;
 
     std::set<TypeTag> field_tags;
 
@@ -288,6 +287,7 @@ const ZAMStmt ZAMCompiler::CompileRecFieldUpdates(const RecordFieldUpdates* e) {
     else
         op = OP_REC_ADD_FIELDS_VV;
 
+    auto lhs = e->GetOp1()->AsNameExpr();
     auto z = GenInst(op, lhs, rhs);
     z.aux = aux;
 
@@ -1267,10 +1267,11 @@ const ZAMStmt ZAMCompiler::ConstructSet(const NameExpr* n, const Expr* e) {
     return AddInst(z);
 }
 
-const ZAMStmt ZAMCompiler::ConstructRecord(const NameExpr* n, const Expr* e) {
+const ZAMStmt ZAMCompiler::ConstructRecord(const NameExpr* n, const Expr* e, bool is_from_rec) {
-    ASSERT(e->Tag() == EXPR_RECORD_CONSTRUCTOR);
+    auto rec_e = is_from_rec ? e->GetOp1().get() : e;
-    auto rc = static_cast<const RecordConstructorExpr*>(e);
+    ASSERT(rec_e->Tag() == EXPR_RECORD_CONSTRUCTOR);
-    auto rt = e->GetType()->AsRecordType();
+    auto rc = static_cast<const RecordConstructorExpr*>(rec_e);
+    auto rt = rec_e->GetType()->AsRecordType();
 
     auto aux = InternalBuildVals(rc->Op().get());
 
@@ -1280,7 +1281,7 @@ const ZAMStmt ZAMCompiler::ConstructRecord(const NameExpr* n, const Expr* e) {
     // constructor.
     aux->zvec.resize(rt->NumFields());
 
-    if ( pfs->HasSideEffects(SideEffectsOp::CONSTRUCTION, e->GetType()) )
+    if ( pfs->HasSideEffects(SideEffectsOp::CONSTRUCTION, rec_e->GetType()) )
         aux->can_change_non_locals = true;
 
     ZOp op;
@@ -1345,33 +1346,88 @@ const ZAMStmt ZAMCompiler::ConstructRecord(const NameExpr* n, const Expr* e) {
     else
         op = OP_CONSTRUCT_DIRECT_RECORD_V;
 
-    ZInstI z = network_time_index >= 0 ? GenInst(op, n, network_time_index) : GenInst(op, n);
+    ZInstI z;
+
+    if ( is_from_rec ) {
+        switch ( op ) {
+            case OP_CONSTRUCT_KNOWN_RECORD_WITH_NT_VV: op = OP_CONSTRUCT_KNOWN_RECORD_WITH_NT_FROM_VVV; break;
+
+            case OP_CONSTRUCT_KNOWN_RECORD_V: op = OP_CONSTRUCT_KNOWN_RECORD_FROM_VV; break;
+
+            case OP_CONSTRUCT_KNOWN_RECORD_WITH_INITS_AND_NT_VV:
+                op = OP_CONSTRUCT_KNOWN_RECORD_WITH_INITS_AND_NT_FROM_VVV;
+                break;
+
+            case OP_CONSTRUCT_KNOWN_RECORD_WITH_INITS_V:
+                op = OP_CONSTRUCT_KNOWN_RECORD_WITH_INITS_FROM_VV;
+                break;
+
+                // Note, no case for OP_CONSTRUCT_DIRECT_RECORD_V - shouldn't happen
+                // given how we construct ConstructFromRecordExpr's.
+            default: reporter->InternalError("bad op in ZAMCompiler::ConstructRecord");
+        }
+
+        auto cfr = static_cast<const ConstructFromRecordExpr*>(e);
+        auto from_n = cfr->GetOp2()->AsNameExpr();
+        if ( network_time_index >= 0 )
+            z = GenInst(op, n, from_n, network_time_index);
+        else
+            z = GenInst(op, n, from_n);
+
+        aux->lhs_map = cfr->LHSMap();
+        aux->rhs_map = cfr->RHSMap();
+
+        for ( auto i : aux->lhs_map ) {
+            auto& field_t = rt->GetFieldType(i);
+            aux->is_managed.push_back(ZVal::IsManagedType(field_t));
+        }
+    }
+
+    else
+        z = network_time_index >= 0 ? GenInst(op, n, network_time_index) : GenInst(op, n);
 
     z.aux = aux;
-    z.t = e->GetType();
+    z.t = rec_e->GetType();
 
     auto inst = AddInst(z);
 
     // If one of the initialization values is an unspecified vector (which
     // in general we can't know until run-time) then we'll need to
     // "concretize" it. We first see whether this is a possibility, since
-    // it usually isn't, by counting up how many of the record fields are
+    // it usually isn't, by counting up how many of the initialized record
-    // vectors.
+    // fields are vectors.
-    std::vector<int> vector_fields; // holds indices of the vector fields
+
+    // First just gather up the types of all the fields, and their location
+    // in the target.
+    std::vector<std::pair<TypePtr, int>> init_field_types;
+
     for ( int i = 0; i < z.aux->n; ++i ) {
         auto field_ind = map ? (*map)[i] : i;
         auto& field_t = rt->GetFieldType(field_ind);
-        if ( field_t->Tag() == TYPE_VECTOR && field_t->Yield()->Tag() != TYPE_ANY )
+        init_field_types.emplace_back(std::pair<TypePtr, int>{field_t, field_ind});
-            vector_fields.push_back(field_ind);
     }
 
+    if ( is_from_rec )
+        // Need to also check the source record.
+        for ( auto i : aux->lhs_map ) {
+            auto& field_t = rt->GetFieldType(i);
+            init_field_types.emplace_back(std::pair<TypePtr, int>{field_t, i});
+        }
+
+    // Now spin through to find the vector fields.
+
+    std::vector<int> vector_fields; // holds indices of the vector fields
+    for ( auto& ft : init_field_types )
+        if ( ft.first->Tag() == TYPE_VECTOR && ft.first->Yield()->Tag() != TYPE_ANY )
+            vector_fields.push_back(ft.second);
+
     if ( vector_fields.empty() )
         // Common case of no vector fields, we're done.
         return inst;
 
     // Need to add a separate instruction for concretizing the fields.
     z = GenInst(OP_CONCRETIZE_VECTOR_FIELDS_V, n);
-    z.t = e->GetType();
+    z.t = rec_e->GetType();
     int nf = static_cast<int>(vector_fields.size());
     z.aux = new ZInstAux(nf);
     z.aux->elems_has_slots = false; // we're storing field offsets, not slots

src/script_opt/ZAM/Ops.in

@@ -1230,6 +1230,8 @@ eval	ConstructTableOrSetPre()
 
 direct-unary-op Record-Constructor ConstructRecord
 
+direct-unary-op Rec-Construct-With-Rec ConstructRecordFromRecord
+
 macro ConstructRecordPost()
 	auto& r = frame[z.v1].record_val;
 	Unref(r);
@@ -1245,48 +1247,85 @@ type V
 eval	auto init_vals = z.aux->ToZValVecWithMap(frame);
 	ConstructRecordPost()
 
+macro	AssignFromRec()
+	/* The following is defined below, for use by Rec-Assign-Fields */
+	SetUpRecFieldOps(lhs_map)
+	auto is_managed = aux->is_managed;
+	for ( size_t i = 0U; i < n; ++i )
+		{
+		auto rhs_i = rhs->RawField(rhs_map[i]);
+		if ( is_managed[i] )
+			zeek::Ref(rhs_i.ManagedVal());
+		init_vals[lhs_map[i]] = rhs_i;
+		}
+
+op Construct-Known-Record-From
+type VV
+eval	auto init_vals = z.aux->ToZValVecWithMap(frame);
+	AssignFromRec()
+	ConstructRecordPost()
+
+macro DoNetworkTimeInit(slot)
+	init_vals[slot] = ZVal(run_state::network_time);
+
 op Construct-Known-Record-With-NT
 type VV
 eval	auto init_vals = z.aux->ToZValVecWithMap(frame);
-	ASSERT(! init_vals[z.v2]);
+	DoNetworkTimeInit(z.v2)
-	init_vals[z.v2] = ZVal(run_state::network_time);
 	ConstructRecordPost()
 
+op Construct-Known-Record-With-NT-From
+type VVV
+eval	auto init_vals = z.aux->ToZValVecWithMap(frame);
+	DoNetworkTimeInit(z.v3)
+	AssignFromRec()
+	ConstructRecordPost()
+
+macro GenInits()
+	auto init_vals = z.aux->ToZValVecWithMap(frame);
+	for ( auto& fi : *z.aux->field_inits )
+		init_vals[fi.first] = fi.second->Generate();
+
 op Construct-Known-Record-With-Inits
 type V
-eval	auto init_vals = z.aux->ToZValVecWithMap(frame);
+eval	GenInits()
-	for ( auto& fi : *z.aux->field_inits )
+	ConstructRecordPost()
-		init_vals[fi.first] = fi.second->Generate();
+
+op Construct-Known-Record-With-Inits-From
+type VV
+eval	GenInits()
+	AssignFromRec()
 	ConstructRecordPost()
 
 op Construct-Known-Record-With-Inits-And-NT
 type VV
-eval	auto init_vals = z.aux->ToZValVecWithMap(frame);
+eval	GenInits()
-	for ( auto& fi : *z.aux->field_inits )
+	DoNetworkTimeInit(z.v2)
-		init_vals[fi.first] = fi.second->Generate();
-	ASSERT(! init_vals[z.v2]);
-	init_vals[z.v2] = ZVal(run_state::network_time);
 	ConstructRecordPost()
 
-macro SetUpRecFieldOps()
+op Construct-Known-Record-With-Inits-And-NT-From
+type VVV
+eval	GenInits()
+	DoNetworkTimeInit(z.v3)
+	AssignFromRec()
+	ConstructRecordPost()
+
+macro SetUpRecFieldOps(which_lhs_map)
 	auto lhs = frame[z.v1].record_val;
 	auto rhs = frame[z.v2].record_val;
 	auto aux = z.aux;
-	auto& lhs_map = aux->map;
+	auto& lhs_map = aux->which_lhs_map;
 	auto& rhs_map = aux->rhs_map;
 	auto n = rhs_map.size();
 
 op Rec-Assign-Fields
 op1-read
 type VV
-eval	SetUpRecFieldOps()
+eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
 		lhs->RawOptField(lhs_map[i]) = rhs->RawField(rhs_map[i]);
 
-op Rec-Assign-Fields-Managed
+macro DoManagedRecAssign()
-op1-read
-type VV
-eval	SetUpRecFieldOps()
 	auto is_managed = aux->is_managed;
 	for ( size_t i = 0U; i < n; ++i )
 		if ( is_managed[i] )
@@ -1300,11 +1339,16 @@ eval	SetUpRecFieldOps()
 			}
 		else
 			lhs->RawOptField(lhs_map[i]) = rhs->RawField(rhs_map[i]);
+op Rec-Assign-Fields-Managed
+op1-read
+type VV
+eval	SetUpRecFieldOps(map)
+	DoManagedRecAssign()
 
 op Rec-Assign-Fields-All-Managed
 op1-read
 type VV
-eval	SetUpRecFieldOps()
+eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
 		{
 		auto& lhs_i = lhs->RawOptField(lhs_map[i]);
@@ -1318,21 +1362,21 @@ eval	SetUpRecFieldOps()
 op Rec-Add-Int-Fields
 op1-read
 type VV
-eval	SetUpRecFieldOps()
+eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
 		lhs->RawField(lhs_map[i]).int_val += rhs->RawField(rhs_map[i]).int_val;
 
 op Rec-Add-Double-Fields
 op1-read
 type VV
-eval	SetUpRecFieldOps()
+eval	SetUpRecFieldOps(map)
 	for ( size_t i = 0U; i < n; ++i )
 		lhs->RawField(lhs_map[i]).double_val += rhs->RawField(rhs_map[i]).double_val;
 
 op Rec-Add-Fields
 op1-read
 type VV
-eval	SetUpRecFieldOps()
+eval	SetUpRecFieldOps(map)
 	auto& types = aux->types;
 	for ( size_t i = 0U; i < n; ++i )
 		{

src/script_opt/ZAM/ZInst.h

@@ -493,6 +493,13 @@ public:
     // the above) and a RHS one.
     std::vector<int> rhs_map;
 
+    // ... and the following when we need *three* (for constructing certain
+    // types of records). We could hack it in by adding onto "map" but
+    // this is cleaner, and we're not really concerned with the size of
+    // ZAM auxiliary information as it's not that commonly used, and doesn't
+    // grow during execution.
+    std::vector<int> lhs_map;
+
     // For operations that need to track types corresponding to other vectors.
     std::vector<TypePtr> types;