From 8c64ba6907d8c2cd8b204a9cf0ac3a73f4034487 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Sun, 10 Jan 2021 17:32:50 -0800 Subject: [PATCH] GH-1296: fix type-checks related to list-type equality List-types as used in composite table/set indices, for example, previously had incorrect same_type() comparisons due to flattening of the list-type into a single type without checking whether the number and kind of types all match. This patch simply removes the flatten_type() call from same_type() since it was already contradicting/preventing a subsequent full-comparison between elements of two TYPE_LISTs. There was also a superfluous special-case of the `in` operator's type-checking for testing whether a record is in a table/set. It's superfluous because the general case will already do the type-checking from MatchesIndex() after first wrapping the record operand in a ListExpr. The previous logic was incorrectly relying on the flatten_type() for testing equality of a record-type against a list-type, whereas the general case correctly normalizes to testing equality of two list-types. The special-cased type-checking logic for assigning a record value to a table index during its initialization similarly needed minor re-organization in order to maintain the same error messages as before. --- src/Expr.cc | 52 +++++++------------ src/Type.cc | 7 +-- .../language.type-check-func-call/out | 5 ++ .../language.type-check-operator-in/out | 8 +++ .../btest/language/type-check-func-call.zeek | 22 ++++++++ .../language/type-check-operator-in.zeek | 32 ++++++++++++ 6 files changed, 88 insertions(+), 38 deletions(-) create mode 100644 testing/btest/Baseline/language.type-check-func-call/out create mode 100644 testing/btest/Baseline/language.type-check-operator-in/out create mode 100644 testing/btest/language/type-check-func-call.zeek create mode 100644 testing/btest/language/type-check-operator-in.zeek diff --git a/src/Expr.cc b/src/Expr.cc index c9cbd10e34..457890bb9f 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -2488,19 +2488,28 @@ ValPtr AssignExpr::InitVal(const zeek::Type* t, ValPtr aggr) const auto index = op1->InitVal(tt->GetIndices().get(), nullptr); - if ( ! same_type(*yt, *op2->GetType(), true) ) + if ( yt->Tag() == TYPE_RECORD ) { - if ( yt->Tag() == TYPE_RECORD && op2->GetType()->Tag() == TYPE_RECORD ) + if ( op2->GetType()->Tag() != TYPE_RECORD ) { - if ( ! record_promotion_compatible(yt->AsRecordType(), - op2->GetType()->AsRecordType()) ) - { - Error("type mismatch in table value initialization: " - "incompatible record types"); - return nullptr; - } + Error(util::fmt("type mismatch in table value initialization: " + "assigning '%s' to table with values of type '%s'", + type_name(op2->GetType()->Tag()), type_name(yt->Tag()))); + return nullptr; } - else + + if ( ! same_type(*yt, *op2->GetType()) && + ! record_promotion_compatible(yt->AsRecordType(), + op2->GetType()->AsRecordType()) ) + { + Error("type mismatch in table value initialization: " + "incompatible record types"); + return nullptr; + } + } + else + { + if ( ! same_type(*yt, *op2->GetType(), true) ) { Error(util::fmt("type mismatch in table value initialization: " "assigning '%s' to table with values of type '%s'", @@ -4086,29 +4095,6 @@ InExpr::InExpr(ExprPtr arg_op1, ExprPtr arg_op2) SetType(base_type(TYPE_BOOL)); } - else if ( op1->GetType()->Tag() == TYPE_RECORD ) - { - if ( op2->GetType()->Tag() != TYPE_TABLE ) - { - op2->GetType()->Error("table/set required"); - SetError(); - } - - else - { - const auto& t1 = op1->GetType(); - const auto& it = op2->GetType()->AsTableType()->GetIndices(); - - if ( ! same_type(t1, it) ) - { - t1->Error("indexing mismatch", op2->GetType().get()); - SetError(); - } - else - SetType(base_type(TYPE_BOOL)); - } - } - else if ( op1->GetType()->Tag() == TYPE_STRING && op2->GetType()->Tag() == TYPE_STRING ) SetType(base_type(TYPE_BOOL)); diff --git a/src/Type.cc b/src/Type.cc index eac80264e8..69fb6602e2 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -1586,11 +1586,8 @@ bool same_type(const Type& arg_t1, const Type& arg_t2, arg_t2.Tag() == TYPE_ANY ) return true; - auto t1 = flatten_type(&arg_t1); - auto t2 = flatten_type(&arg_t2); - - if ( t1 == t2 ) - return true; + auto t1 = &arg_t1; + auto t2 = &arg_t2; if ( t1->Tag() != t2->Tag() ) { diff --git a/testing/btest/Baseline/language.type-check-func-call/out b/testing/btest/Baseline/language.type-check-func-call/out new file mode 100644 index 0000000000..07308933c1 --- /dev/null +++ b/testing/btest/Baseline/language.type-check-func-call/out @@ -0,0 +1,5 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in <...>/type-check-func-call.zeek, line 8 and <...>/type-check-func-call.zeek, line 21: type clash (set[string] and s) +error in <...>/type-check-func-call.zeek, line 21 and <...>/type-check-func-call.zeek, line 8: type mismatch (s and set[string]) +error in <...>/type-check-func-call.zeek, line 21: argument type mismatch in function call (sort_set(s)) +warning in <...>/type-check-func-call.zeek, line 21: expression value ignored (sort_set(s)) diff --git a/testing/btest/Baseline/language.type-check-operator-in/out b/testing/btest/Baseline/language.type-check-operator-in/out new file mode 100644 index 0000000000..68114462b3 --- /dev/null +++ b/testing/btest/Baseline/language.type-check-operator-in/out @@ -0,0 +1,8 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in int and <...>/type-check-operator-in.zeek, line 30: arithmetic mixed with non-arithmetic (int and myrec) +error in <...>/type-check-operator-in.zeek, line 30 and int: type mismatch (myrec and int) +error in <...>/type-check-operator-in.zeek, line 30: not an index type (myrec in asdf) +error in <...>/type-check-operator-in.zeek, line 14 and <...>/type-check-operator-in.zeek, line 31: indexing mismatch (list of string,MyRec and myrec) +error in <...>/type-check-operator-in.zeek, line 31: not an index type (myrec in string_records) +error in <...>/type-check-operator-in.zeek, line 15 and <...>/type-check-operator-in.zeek, line 32: indexing mismatch (list of MyRec,string and myrec) +error in <...>/type-check-operator-in.zeek, line 32: not an index type (myrec in record_strings) diff --git a/testing/btest/language/type-check-func-call.zeek b/testing/btest/language/type-check-func-call.zeek new file mode 100644 index 0000000000..f1951b4e1a --- /dev/null +++ b/testing/btest/language/type-check-func-call.zeek @@ -0,0 +1,22 @@ +# @TEST-EXEC-FAIL: zeek -b %INPUT >out 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out + +# @TEST-DOC: Test for type-checking of function call arguments. + +global s: set[string, string]; + +function sort_set(s: set[string]): vector of string + { + local v: vector of string = vector(); + + for ( e in s ) + v += e; + + sort(v, strcmp); + return v; + } + +add s["hi", "there"]; +# This sort_set call should warn about mismatched table/set types. +sort_set(s); + diff --git a/testing/btest/language/type-check-operator-in.zeek b/testing/btest/language/type-check-operator-in.zeek new file mode 100644 index 0000000000..d5f340320f --- /dev/null +++ b/testing/btest/language/type-check-operator-in.zeek @@ -0,0 +1,32 @@ +# @TEST-EXEC-FAIL: zeek -b %INPUT >out 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out + +# @TEST-DOC: Test for type-checking of `in` operator. + +type MyRec: record { + a: count &default=1; +}; + +local myrec = MyRec(); +local strings: set[string] = set(); +local records: set[MyRec] = set(); +local string_counts: set[string, count] = set(); +local string_records: set[string, MyRec] = set(); +local record_strings: set[MyRec, string] = set(); + +# These are all valid. +["asdf"] in strings; +["hi", 0] in string_counts; +myrec in records; +[myrec] in records; +MyRec() in records; +[$a = 2] in records; +[MyRec()] in records; +[[$a = 2]] in records; +["hi", myrec] in string_records; + +# All below should fail type-checking. + +myrec in "asdf"; +myrec in string_records; +myrec in record_strings;