Merge remote-tracking branch 'origin/topic/jsiwek/gh-251-revert-absolute-value-coercion'

* origin/topic/jsiwek/gh-251-revert-absolute-value-coercion:
  GH-251 (revert): remove coercion-to-signed-integer for |x| expressions

CHANGES

@@ -1,4 +1,21 @@
 
+3.3.0-dev.479 | 2020-10-23 12:25:15 -0700
+
+  * GH-251 (revert): remove coercion-to-signed-integer for |x| expressions (Jon Siwek, Corelight)
+
+    For `|x|`, where `x` is an expression with an integral result, an
+    implicit coercion of that result into signed `int` type no longer takes
+    place.
+
+    This was actually the behavior before Zeek 3.0 as well, but the attempt
+    to prevent mistakes that easily result from integer literals in Zeek
+    being unsigned like `|5 - 9|` causing an overflow/wraparound and
+    yielding a very large number is not generally consistent since overflows
+    are still generally able to happen in other ways and also in other
+    contexts besides just absolute-values.  So the preference was to revert
+    to a behavior that favors consistency.  For reference, see
+    https://github.com/zeek/zeek/pull/251#issuecomment-713956976
+
 3.3.0-dev.476 | 2020-10-22 15:59:56 -0400
 
   * Add an option to ignore packets sourced from particular subnets.

NEWS

@@ -98,6 +98,17 @@ Changed Functionality
   not pre-configured to listen for incoming connections from other cluster
   nodes.
 
+- The ``|x|`` operator, where ``x`` is an expression with an integral result,
+  no longer performs an implicit coercion of that result into a signed
+  ``int`` type.  This was actually the behavior before Zeek 3.0 as well, but
+  the attempt to prevent mistakes that easily result from integer literals in
+  Zeek being unsigned like ``|5 - 9|`` causing an overflow/wraparound and
+  yielding a very large number is not generally consistent since overflows
+  are still generally able to happen in other ways and also in other contexts
+  besides just the absolute-value operator.  So the preference was to revert
+  to a behavior that favors consistency.  For reference, see
+  https://github.com/zeek/zeek/pull/251#issuecomment-713956976
+
 Removed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-3.3.0-dev.476
+3.3.0-dev.479

doc

@@ -1 +1 @@
-Subproject commit 44d5cef4f940f313b4e73a516cbe587621d652de
+Subproject commit 692fdef75227cb7a6b7450c024fab1d27096562c

src/parse.y

@@ -767,10 +767,6 @@ expr:
 			{
 			zeek::detail::set_location(@1, @3);
 			zeek::detail::ExprPtr e{zeek::AdoptRef{}, $2};
-
-			if ( zeek::IsIntegral(e->GetType()->Tag()) )
-				e = zeek::make_intrusive<zeek::detail::ArithCoerceExpr>(std::move(e), zeek::TYPE_INT);
-
 			$$ = new zeek::detail::SizeExpr(std::move(e));
 			}
 

testing/btest/Baseline/language.sizeof/output

@@ -1,8 +1,10 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
 IPv4 Address 1.2.3.4: 32
 IPv6 Address ::1: 128
 Boolean T: 1
 Count 10: 10
-Expr: 4
+Expr: 18446744073709551612
+Signed Expr: 4
 Double -1.23: 1.230000
 Enum ENUM3: 2
 File 21.000000

testing/btest/language/sizeof.zeek

@@ -64,9 +64,14 @@ print fmt("Boolean %s: %d", b, |b|);
 # Size of count: identity.
 print fmt("Count %s: %d", c, |c|);
 
-# Size of integral arithmetic expression should coerce to int before absolute
-# value operation to help prevent common unsigned int overflow situations.
+# Integer literals that lack a "+" or "-" modifier are of the unsigned "count"
+# type, so this wraps to a very large number.  It may be more intuitive if it
+# were to coerce to a signed integer, but it can also be more favorable to
+# simply have consistent behavior across arbitrary arithmetic expressions even
+# if that may result in occassional, unintended overflow/wrapping.
 print fmt("Expr: %d", |5 - 9|);
+# Same arithmetic on signed integers is likely what's originally intended.
+print fmt("Signed Expr: %d", |+5 - +9|);
 
 # Size of double: returns absolute value.
 print fmt("Double %s: %f", d, |d|);