"add" and "delete" are now expressions rather than statements

src/Expr.cc

@@ -47,6 +47,8 @@ const char* expr_name(ExprTag t) {
         "-",
         "+",
         "-",
+        "add ",
+        "delete ",
         "+=",
         "-=",
         "*",
@@ -216,9 +218,13 @@ bool Expr::CanAdd() const { return false; }
 
 bool Expr::CanDel() const { return false; }
 
-void Expr::Add(Frame* /* f */) { Internal("Expr::Add called"); }
+TypePtr Expr::AddType() const { return nullptr; }
 
-void Expr::Delete(Frame* /* f */) { Internal("Expr::Delete called"); }
+TypePtr Expr::DelType() const { return nullptr; }
+
+ValPtr Expr::Add(Frame* /* f */) { Internal("Expr::Add called"); }
+
+ValPtr Expr::Delete(Frame* /* f */) { Internal("Expr::Delete called"); }
 
 ExprPtr Expr::MakeLvalue() {
     if ( ! IsError() )
@@ -421,8 +427,9 @@ bool NameExpr::CanDel() const {
     return GetType()->Tag() == TYPE_TABLE || GetType()->Tag() == TYPE_VECTOR;
 }
 
-void NameExpr::Delete(Frame* f) {
+ValPtr NameExpr::Delete(Frame* f) {
-    if ( auto v = Eval(f) ) {
+    auto v = Eval(f);
+    if ( v ) {
         if ( GetType()->Tag() == TYPE_TABLE )
             v->AsTableVal()->RemoveAll();
         else if ( GetType()->Tag() == TYPE_VECTOR )
@@ -430,6 +437,7 @@ void NameExpr::Delete(Frame* f) {
         else
             RuntimeError("delete unsupported");
     }
+    return v;
 }
 
 // This isn't in-lined to avoid needing to pull in ID.h.
@@ -1398,6 +1406,24 @@ void AddExpr::Canonicalize() {
         SwapOps();
 }
 
+AggrAddExpr::AggrAddExpr(ExprPtr _op) : AggrAddDelExpr(EXPR_AGGR_ADD, std::move(_op)) {
+    if ( ! op->IsError() && ! op->CanAdd() )
+        ExprError("illegal add expression");
+
+    SetType(op->AddType());
+}
+
+ValPtr AggrAddExpr::Eval(Frame* f) const { return op->Add(f); }
+
+AggrDelExpr::AggrDelExpr(ExprPtr _op) : AggrAddDelExpr(EXPR_AGGR_DEL, std::move(_op)) {
+    if ( ! op->IsError() && ! op->CanDel() )
+        Error("illegal delete expression");
+
+    SetType(op->DelType());
+}
+
+ValPtr AggrDelExpr::Eval(Frame* f) const { return op->Delete(f); }
+
 // True if we should treat LHS += RHS as add-every-element-of-RHS-to-LHS.
 // False for the alternative, add-RHS-as-one-element-to-LHS.
 //
@@ -2501,46 +2527,52 @@ bool IndexExpr::CanDel() const {
     return op1->GetType()->Tag() == TYPE_TABLE;
 }
 
-void IndexExpr::Add(Frame* f) {
+ValPtr IndexExpr::Add(Frame* f) {
     if ( IsError() )
-        return;
+        return nullptr;
 
     auto v1 = op1->Eval(f);
 
     if ( ! v1 )
-        return;
+        return nullptr;
 
     auto v2 = op2->Eval(f);
 
     if ( ! v2 )
-        return;
+        return nullptr;
 
     bool iterators_invalidated = false;
     v1->AsTableVal()->Assign(std::move(v2), nullptr, true, &iterators_invalidated);
 
     if ( iterators_invalidated )
         reporter->ExprRuntimeWarning(this, "possible loop/iterator invalidation");
+
+    // In the future we could return a value, such as v1, here.
+    return nullptr;
 }
 
-void IndexExpr::Delete(Frame* f) {
+ValPtr IndexExpr::Delete(Frame* f) {
     if ( IsError() )
-        return;
+        return nullptr;
 
     auto v1 = op1->Eval(f);
 
     if ( ! v1 )
-        return;
+        return nullptr;
 
     auto v2 = op2->Eval(f);
 
     if ( ! v2 )
-        return;
+        return nullptr;
 
     bool iterators_invalidated = false;
-    v1->AsTableVal()->Remove(*v2, true, &iterators_invalidated);
+    auto removed = v1->AsTableVal()->Remove(*v2, true, &iterators_invalidated);
 
     if ( iterators_invalidated )
         reporter->ExprRuntimeWarning(this, "possible loop/iterator invalidation");
+
+    // In the future we could return a value, such as removed, here.
+    return nullptr;
 }
 
 ExprPtr IndexExpr::MakeLvalue() {
@@ -2761,13 +2793,24 @@ void FieldExpr::Assign(Frame* f, ValPtr v) {
     if ( IsError() )
         return;
 
-    if ( auto op_v = op->Eval(f) ) {
+    Assign(op->Eval(f), v);
-        RecordVal* r = op_v->AsRecordVal();
-        r->Assign(field, std::move(v));
-    }
 }
 
-void FieldExpr::Delete(Frame* f) { Assign(f, nullptr); }
+void FieldExpr::Assign(ValPtr lhs, ValPtr rhs) {
+    if ( lhs )
+        lhs->AsRecordVal()->Assign(field, std::move(rhs));
+}
+
+ValPtr FieldExpr::Delete(Frame* f) {
+    auto op_v = op->Eval(f);
+    if ( ! op_v )
+        return nullptr;
+
+    auto former = op_v->AsRecordVal()->GetField(field);
+    Assign(op_v, nullptr);
+    // In the future we could return a value, such as former, here.
+    return nullptr;
+}
 
 ValPtr FieldExpr::Fold(Val* v) const {
     if ( const auto& result = v->AsRecordVal()->GetField(field) )

src/Expr.h

@@ -46,6 +46,8 @@ enum ExprTag : int {
     EXPR_NEGATE,
     EXPR_ADD,
     EXPR_SUB,
+    EXPR_AGGR_ADD,
+    EXPR_AGGR_DEL,
     EXPR_ADD_TO,
     EXPR_REMOVE_FROM,
     EXPR_TIMES,
@@ -204,8 +206,12 @@ public:
     virtual bool CanAdd() const;
     virtual bool CanDel() const;
 
-    virtual void Add(Frame* f);    // perform add operation
+    // The types associated with those operations.
-    virtual void Delete(Frame* f); // perform delete operation
+    virtual TypePtr AddType() const;
+    virtual TypePtr DelType() const;
+
+    virtual ValPtr Add(Frame* f);    // perform add operation
+    virtual ValPtr Delete(Frame* f); // perform delete operation
 
     // Return the expression converted to L-value form.  If expr
     // cannot be used as an L-value, reports an error and returns
@@ -421,7 +427,7 @@ public:
     explicit NameExpr(IDPtr id, bool const_init = false);
 
     bool CanDel() const override;
-    void Delete(Frame* f) override;
+    ValPtr Delete(Frame* f) override;
 
     ID* Id() const { return id.get(); }
     const IDPtr& IdPtr() const;
@@ -700,6 +706,41 @@ protected:
     ExprPtr BuildSub(const ExprPtr& op1, const ExprPtr& op2);
 };
 
+// A helper class that enables us to factor some common code.
+class AggrAddDelExpr : public UnaryExpr {
+public:
+    explicit AggrAddDelExpr(ExprTag _tag, ExprPtr _e) : UnaryExpr(_tag, std::move(_e)) {}
+
+    bool IsPure() const override { return false; }
+
+    // Optimization-related:
+    bool IsReduced(Reducer* c) const override { return HasReducedOps(c); }
+    bool HasReducedOps(Reducer* c) const override { return op->HasReducedOps(c); }
+    ExprPtr Reduce(Reducer* c, StmtPtr& red_stmt) override;
+};
+
+class AggrAddExpr final : public AggrAddDelExpr {
+public:
+    explicit AggrAddExpr(ExprPtr e);
+
+    // Optimization-related:
+    ExprPtr Duplicate() override;
+
+protected:
+    ValPtr Eval(Frame* f) const override;
+};
+
+class AggrDelExpr final : public AggrAddDelExpr {
+public:
+    explicit AggrDelExpr(ExprPtr e);
+
+    // Optimization-related:
+    ExprPtr Duplicate() override;
+
+protected:
+    ValPtr Eval(Frame* f) const override;
+};
+
 class AddToExpr final : public BinaryExpr {
 public:
     AddToExpr(ExprPtr op1, ExprPtr op2);
@@ -976,8 +1017,8 @@ public:
     bool CanAdd() const override;
     bool CanDel() const override;
 
-    void Add(Frame* f) override;
+    ValPtr Add(Frame* f) override;
-    void Delete(Frame* f) override;
+    ValPtr Delete(Frame* f) override;
 
     void Assign(Frame* f, ValPtr v) override;
     ExprPtr MakeLvalue() override;
@@ -1077,7 +1118,7 @@ public:
     bool CanDel() const override;
 
     void Assign(Frame* f, ValPtr v) override;
-    void Delete(Frame* f) override;
+    ValPtr Delete(Frame* f) override;
 
     ExprPtr MakeLvalue() override;
 
@@ -1085,6 +1126,7 @@ public:
     ExprPtr Duplicate() override;
 
 protected:
+    void Assign(ValPtr lhs, ValPtr rhs);
     ValPtr Fold(Val* v) const override;
 
     void ExprDescribe(ODesc* d) const override;

src/parse.y

@@ -37,7 +37,7 @@
 %token TOK_NO_TEST
 
 %left ','
-%right '=' TOK_ADD_TO TOK_REMOVE_FROM
+%right '=' TOK_ADD_TO TOK_REMOVE_FROM TOK_ADD TOK_DELETE
 %right '?' ':'
 %left TOK_OR_OR
 %left TOK_AND_AND
@@ -508,6 +508,18 @@ expr:
 			$$ = new CloneExpr({AdoptRef{}, $3});
 			}
 
+	|	TOK_ADD expr
+			{
+			set_location(@1, @2);
+			$$ = new AggrAddExpr({AdoptRef{}, $2});
+			}
+
+	|	TOK_DELETE expr
+			{
+			set_location(@1, @2);
+			$$ = new AggrDelExpr({AdoptRef{}, $2});
+			}
+
 	|	TOK_INCR expr
 			{
 			set_location(@1, @2);
@@ -1934,22 +1946,6 @@ stmt:
 			    script_coverage_mgr.AddStmt($$);
 			}
 
-	|	TOK_ADD expr ';' opt_no_test
-			{
-			set_location(@1, @3);
-			$$ = new AddStmt({AdoptRef{}, $2});
-			if ( ! $4 )
-			    script_coverage_mgr.AddStmt($$);
-			}
-
-	|	TOK_DELETE expr ';' opt_no_test
-			{
-			set_location(@1, @3);
-			$$ = new DelStmt({AdoptRef{}, $2});
-			if ( ! $4 )
-			    script_coverage_mgr.AddStmt($$);
-			}
-
 	|	TOK_LOCAL local_id opt_type init_class opt_init opt_attr ';' opt_no_test
 			{
 			set_location(@1, @7);

src/script_opt/Expr.cc

@@ -723,6 +723,21 @@ ExprPtr AddExpr::BuildSub(const ExprPtr& op1, const ExprPtr& op2) {
     return with_location_of(make_intrusive<SubExpr>(op1, rhs), this);
 }
 
+ExprPtr AggrAddDelExpr::Reduce(Reducer* c, StmtPtr& red_stmt) {
+    if ( type )
+        return UnaryExpr::Reduce(c, red_stmt);
+
+    if ( c->Optimizing() )
+        op = c->UpdateExpr(op);
+
+    red_stmt = op->ReduceToSingletons(c);
+    return ThisPtr();
+}
+
+ExprPtr AggrAddExpr::Duplicate() { return SetSucc(new AggrAddExpr(op->Duplicate())); }
+
+ExprPtr AggrDelExpr::Duplicate() { return SetSucc(new AggrDelExpr(op->Duplicate())); }
+
 ExprPtr AddToExpr::Duplicate() {
     auto op1_d = op1->Duplicate();
     auto op2_d = op2->Duplicate();

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

@@ -1,2 +1,2 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-error in <...>/wrong-delete-field.zeek, line 10: illegal delete statement (delete x$a)
+error in <...>/wrong-delete-field.zeek, line 10: illegal delete expression (delete x$a)