Merge remote-tracking branch 'origin/topic/zeke/gh-1890'

* origin/topic/zeke/gh-1890:
  Consistently warn about mixing vector and scalar operand depreciaton

src/Expr.cc

@@ -13,11 +13,13 @@
 #include "zeek/Hash.h"
 #include "zeek/IPAddr.h"
 #include "zeek/RE.h"
+#include "zeek/Reporter.h"
 #include "zeek/RunState.h"
 #include "zeek/Scope.h"
 #include "zeek/Stmt.h"
 #include "zeek/Traverse.h"
 #include "zeek/Trigger.h"
+#include "zeek/Type.h"
 #include "zeek/broker/Data.h"
 #include "zeek/digest.h"
 #include "zeek/module_util.h"
@@ -1219,9 +1221,6 @@ void BinaryExpr::PromoteOps(TypeTag t)
 	if ( is_vec2 )
 		bt2 = op2->GetType()->AsVectorType()->Yield()->Tag();
 
-	if ( (is_vec1 || is_vec2) && ! (is_vec1 && is_vec2) )
-		reporter->Warning("mixing vector and scalar operands is deprecated");
-
 	if ( bt1 != t )
 		op1 = make_intrusive<ArithCoerceExpr>(op1, t);
 	if ( bt2 != t )
@@ -1249,6 +1248,25 @@ void BinaryExpr::PromoteForInterval(ExprPtr& op)
 		op = make_intrusive<ArithCoerceExpr>(op, TYPE_DOUBLE);
 	}
 
+bool BinaryExpr::IsScalarAggregateOp() const
+	{
+	const bool is_vec1 = IsAggr(op1->GetType()->Tag()) || is_list(op1);
+	const bool is_vec2 = IsAggr(op2->GetType()->Tag()) || is_list(op2);
+	const bool either_vec = is_vec1 || is_vec2;
+	const bool both_vec = is_vec1 && is_vec2;
+
+	return either_vec && ! both_vec;
+	}
+
+void BinaryExpr::CheckScalarAggOp() const
+	{
+	if ( ! IsError() && IsScalarAggregateOp() )
+		{
+		reporter->Warning("mixing vector and scalar operands is deprecated (%s) (%s)",
+		                  type_name(op1->GetType()->Tag()), type_name(op2->GetType()->Tag()));
+		}
+	}
+
 CloneExpr::CloneExpr(ExprPtr arg_op) : UnaryExpr(EXPR_CLONE, std::move(arg_op))
 	{
 	if ( IsError() )
@@ -1520,6 +1538,8 @@ AddExpr::AddExpr(ExprPtr arg_op1, ExprPtr arg_op2)
 	else
 		ExprError("requires arithmetic operands");
 
+	CheckScalarAggOp();
+
 	if ( base_result_type )
 		{
 		if ( is_vector(op1) || is_vector(op2) )
@@ -1652,6 +1672,8 @@ SubExpr::SubExpr(ExprPtr arg_op1, ExprPtr arg_op2)
 	else
 		ExprError("requires arithmetic operands");
 
+	CheckScalarAggOp();
+
 	if ( base_result_type )
 		{
 		if ( is_vector(op1) || is_vector(op2) )
@@ -1728,6 +1750,8 @@ TimesExpr::TimesExpr(ExprPtr arg_op1, ExprPtr arg_op2)
 		PromoteType(max_type(bt1, bt2), is_vector(op1) || is_vector(op2));
 	else
 		ExprError("requires arithmetic operands");
+
+	CheckScalarAggOp();
 	}
 
 void TimesExpr::Canonicize()
@@ -1776,6 +1800,8 @@ DivideExpr::DivideExpr(ExprPtr arg_op1, ExprPtr arg_op2)
 
 	else
 		ExprError("requires arithmetic operands");
+
+	CheckScalarAggOp();
 	}
 
 ValPtr DivideExpr::AddrFold(Val* v1, Val* v2) const
@@ -1823,6 +1849,8 @@ ModExpr::ModExpr(ExprPtr arg_op1, ExprPtr arg_op2)
 		PromoteType(max_type(bt1, bt2), is_vector(op1) || is_vector(op2));
 	else
 		ExprError("requires integral operands");
+
+	CheckScalarAggOp();
 	}
 
 BoolExpr::BoolExpr(BroExprTag arg_tag, ExprPtr arg_op1, ExprPtr arg_op2)
@@ -2086,6 +2114,8 @@ EqExpr::EqExpr(BroExprTag arg_tag, ExprPtr arg_op1, ExprPtr arg_op2)
 
 	else
 		ExprError("type clash in comparison");
+
+	CheckScalarAggOp();
 	}
 
 void EqExpr::Canonicize()
@@ -2166,6 +2196,8 @@ RelExpr::RelExpr(BroExprTag arg_tag, ExprPtr arg_op1, ExprPtr arg_op2)
 	else if ( bt1 != TYPE_TIME && bt1 != TYPE_INTERVAL && bt1 != TYPE_PORT && bt1 != TYPE_ADDR &&
 	          bt1 != TYPE_STRING )
 		ExprError("illegal comparison");
+
+	CheckScalarAggOp();
 	}
 
 void RelExpr::Canonicize()

src/Expr.h

@@ -622,6 +622,14 @@ protected:
 
 	void ExprDescribe(ODesc* d) const override;
 
+	// Reports on if this BinaryExpr involves a scalar and aggregate
+	// type (vec, list, table, record).
+	bool IsScalarAggregateOp() const;
+
+	// Warns about depreciated scalar vector operations like `[1, 2,
+	// 3] == 1` or `["a", "b", "c"] + "a"`.
+	void CheckScalarAggOp() const;
+
 	ExprPtr op1;
 	ExprPtr op2;
 	};
@@ -1801,5 +1809,16 @@ inline bool is_vector(const ExprPtr& e)
 	return is_vector(e.get());
 	}
 
+// True if the given Expr* has a list type
+inline bool is_list(Expr* e)
+	{
+	return e->GetType()->Tag() == TYPE_LIST;
+	}
+
+inline bool is_list(const ExprPtr& e)
+	{
+	return is_list(e.get());
+	}
+
 	} // namespace detail
 	} // namespace zeek

testing/btest/Baseline/core.scalar-vector/out

@@ -0,0 +1,16 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+warning in <...>/scalar-vector.zeek, line 7: mixing vector and scalar operands is deprecated (string) (vector)
+warning in <...>/scalar-vector.zeek, line 8: mixing vector and scalar operands is deprecated (vector) (string)
+warning in <...>/scalar-vector.zeek, line 9: mixing vector and scalar operands is deprecated (string) (vector)
+warning in <...>/scalar-vector.zeek, line 13: mixing vector and scalar operands is deprecated (count) (vector)
+warning in <...>/scalar-vector.zeek, line 14: mixing vector and scalar operands is deprecated (count) (vector)
+warning in <...>/scalar-vector.zeek, line 15: mixing vector and scalar operands is deprecated (vector) (count)
+warning in <...>/scalar-vector.zeek, line 16: mixing vector and scalar operands is deprecated (vector) (count)
+[F, T, F]
+[aa, ba, ca]
+[aa, ab, ac]
+[F, T, F]
+[2, 4, 6]
+[1, 0, 1]
+[0, 1, 1]
+[1, 2, 3, 1]

testing/btest/core/scalar-vector.zeek

@@ -0,0 +1,20 @@
+# @TEST-EXEC: zeek -b %INPUT > out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER="$SCRIPTS/diff-remove-abspath" btest-diff out
+
+event zeek_init()
+      {
+      const sv = vector("a", "b", "c");
+      print sv == "b";
+      print sv + "a";
+      print "a" + sv;
+
+
+      const nv = vector(1, 2, 3);
+      print nv == 2;
+      print nv * 2;
+      print nv % 2;
+      print nv / 2;
+
+      const also_nv = nv += 1;
+      print nv;
+      }