From 06061a357960bf2a1e2d3cf3be5da316aedf41ed Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Thu, 9 Jan 2025 17:40:37 -0500 Subject: [PATCH] Make empty container expansion more intuitive Closes #3933 There are a couple ways to get a list, sometimes with `{}`, sometimes with `[]`. You can use these to append to sets and vectors. But, sometimes it's useful to expand the elements of a list, like if you pass in a global when appending a set. So, those elements are evaluated and possibly change the list. This can, however, end up with an empty list that seems like it should be impactful for construction. It seems useful to say that an empty list is intentional, since the initial check is not able to catch those cases. So, keep that in the list if it's empty. --- src/Expr.cc | 6 ++++ .../Baseline/language.init-mismatch/.stderr | 36 +++++++++++-------- testing/btest/Baseline/language.set/out | 2 ++ testing/btest/language/init-mismatch.zeek | 5 +++ testing/btest/language/set.zeek | 11 ++++++ 5 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index 88c2bbd759..bfb6b17be6 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -3139,6 +3139,12 @@ static bool expand_op_elem(ListExprPtr elems, ExprPtr elem, TypePtr t) { return false; } + // If this expands to nothing, assume that is important and don't expand. + if ( v->AsTableVal()->Size() == 0 ) { + elems->Append(elem); + return false; + } + for ( auto& s_elem : v->AsTableVal()->ToMap() ) { auto c_elem = with_location_of(make_intrusive(s_elem.first), elem); elems->Append(expand_one_elem(index_exprs, yield, c_elem, set_offset)); diff --git a/testing/btest/Baseline/language.init-mismatch/.stderr b/testing/btest/Baseline/language.init-mismatch/.stderr index 294a1efe24..b9f4335a16 100644 --- a/testing/btest/Baseline/language.init-mismatch/.stderr +++ b/testing/btest/Baseline/language.init-mismatch/.stderr @@ -5,17 +5,25 @@ error in <...>/init-mismatch.zeek, line 7: Initialization not preceded by =<...> error in <...>/init-mismatch.zeek, line 13: different number of indices (list of count,count and list of count,count,count) error in <...>/init-mismatch.zeek, line 14: table constructor element lacks '=' structure (bar) error in <...>/init-mismatch.zeek, line 17: empty list in untyped initialization () -error in <...>/init-mismatch.zeek, line 23: cannot expand constructor elements using a value that depends on local variables (subnets) -error in <...>/init-mismatch.zeek, line 23: type clash in assignment (my_subnets = set(foo, subnets)) -error in <...>/init-mismatch.zeek, line 26: requires arithmetic operands (c + 2, 4) -error in <...>/init-mismatch.zeek, line 27: constructor list not allowed for -= operations on vectors (v -= 3, 5) -error in <...>/init-mismatch.zeek, line 29: RHS type mismatch for table/set += (s1 += s2) -error in <...>/init-mismatch.zeek, line 30: RHS type mismatch for table/set -= (s1 -= s2) -error in <...>/init-mismatch.zeek, line 32: table constructor used in a non-table context (3 = F) -error in double and <...>/init-mismatch.zeek, line 32: arithmetic mixed with non-arithmetic (double and 3 = F) -error in <...>/init-mismatch.zeek, line 32 and double: type mismatch (3 = F and double) -error in <...>/init-mismatch.zeek, line 32: inconsistent type in set constructor (set(3 = F)) -error in <...>/init-mismatch.zeek, line 34: not a list of indices (s2) -error in <...>/init-mismatch.zeek, line 34: type clash in assignment (s3 = set(s2)) -error in <...>/init-mismatch.zeek, line 36: pattern += op requires op to be a pattern (p += 3) -error in <...>/init-mismatch.zeek, line 38: illegal table constructor element (1.2.3.4) +error in <...>/init-mismatch.zeek, line 21: type clash (set[count] and set()) +error in <...>/init-mismatch.zeek, line 21: type mismatch (set() and set[count]) +error in <...>/init-mismatch.zeek, line 21: inconsistent type in set constructor (set(set())) +error in <...>/init-mismatch.zeek, line 21: type clash in assignment (s4 = set(set())) +error in <...>/init-mismatch.zeek, line 22: type clash (vector of count and vector()) +error in <...>/init-mismatch.zeek, line 22: type mismatch (vector() and vector of count) +error in <...>/init-mismatch.zeek, line 22: inconsistent type in set constructor (set(vector())) +error in <...>/init-mismatch.zeek, line 22: type clash in assignment (s5 = set(vector())) +error in <...>/init-mismatch.zeek, line 28: cannot expand constructor elements using a value that depends on local variables (subnets) +error in <...>/init-mismatch.zeek, line 28: type clash in assignment (my_subnets = set(foo, subnets)) +error in <...>/init-mismatch.zeek, line 31: requires arithmetic operands (c + 2, 4) +error in <...>/init-mismatch.zeek, line 32: constructor list not allowed for -= operations on vectors (v -= 3, 5) +error in <...>/init-mismatch.zeek, line 34: RHS type mismatch for table/set += (s1 += s2) +error in <...>/init-mismatch.zeek, line 35: RHS type mismatch for table/set -= (s1 -= s2) +error in <...>/init-mismatch.zeek, line 37: table constructor used in a non-table context (3 = F) +error in double and <...>/init-mismatch.zeek, line 37: arithmetic mixed with non-arithmetic (double and 3 = F) +error in <...>/init-mismatch.zeek, line 37 and double: type mismatch (3 = F and double) +error in <...>/init-mismatch.zeek, line 37: inconsistent type in set constructor (set(3 = F)) +error in <...>/init-mismatch.zeek, line 39: not a list of indices (s2) +error in <...>/init-mismatch.zeek, line 39: type clash in assignment (s3 = set(s2)) +error in <...>/init-mismatch.zeek, line 41: pattern += op requires op to be a pattern (p += 3) +error in <...>/init-mismatch.zeek, line 43: illegal table constructor element (1.2.3.4) diff --git a/testing/btest/Baseline/language.set/out b/testing/btest/Baseline/language.set/out index c53cae332c..c3e47d607c 100644 --- a/testing/btest/Baseline/language.set/out +++ b/testing/btest/Baseline/language.set/out @@ -99,3 +99,5 @@ pattern index reduced size (PASS) pattern index iteration (PASS) pattern index JSON roundtrip success (PASS) pattern index JSON roundtrip correct (PASS) +empty set gets added with multiple types (PASS) +empty vector gets added with multiple types (PASS) diff --git a/testing/btest/language/init-mismatch.zeek b/testing/btest/language/init-mismatch.zeek index 2153ca6efd..b260646d37 100644 --- a/testing/btest/language/init-mismatch.zeek +++ b/testing/btest/language/init-mismatch.zeek @@ -16,6 +16,11 @@ global v: vector of count; global p: pattern; global x = { }; +# Issue #3933: For sets that contain a list and another set, expansion should behave +# a bit more conservatively. That means the inner [] are impactful +global s4: set[set[count]] = { [ set() ] }; +global s5: set[vector of count] = { [ vector() ] }; + function foo() { local subnets = { 1.2.3.4/24, 2.3.4.5/5 }; diff --git a/testing/btest/language/set.zeek b/testing/btest/language/set.zeek index 24e2ce402c..c90f7f1db0 100644 --- a/testing/btest/language/set.zeek +++ b/testing/btest/language/set.zeek @@ -294,6 +294,16 @@ function complex_index_type_pattern() test_case( "pattern index JSON roundtrip correct", to_json(s) == to_json(fjr$v) ); } +# Issue 3933: Sets defined with an empty container and {[]} were silently discarding +# the inner set. +function with_empty_containers() + { + local simple_set: set[count, set[count]] = { [ 1, set() ] }; + test_case("empty set gets added with multiple types", |simple_set| == 1); + local simple_vec: set[count, vector of count] = { [ 1, vector() ] }; + test_case("empty vector gets added with multiple types", |simple_vec| == 1); + } + event zeek_init() { basic_functionality(); @@ -301,4 +311,5 @@ event zeek_init() complex_index_type_vector(); complex_index_type_set(); complex_index_type_pattern(); + with_empty_containers(); }