From adf56ef4d8290f206f6d69b9512392be0da6c66c Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 26 Jan 2023 21:14:32 +0100 Subject: [PATCH] Skip somer error reporting when the record type has errors The added test cases around function/event invocations report the following flurry of errors when only the first one is relevant and actionable. There's little use in reporting a mismatch with "error". Squelch them. error in <...>/function-invoke-mismatch-error.zeek, line 8: identifier not defined: MyEnumTypo error in <...>/function-invoke-mismatch-error.zeek, line 12 and error: type mismatch (M::MY_ENUM_A and error) error in <...>/function-invoke-mismatch-error.zeek, line 12: argument type mismatch in function call (M::to_string(M::MY_ENUM_A)) error in <...>/function-invoke-mismatch-error.zeek, line 16 and error: type mismatch (M::MY_ENUM_B and error) error in <...>/function-invoke-mismatch-error.zeek, line 16: argument type mismatch in function call (M::to_string(M::MY_ENUM_B)) error in <...>/function-invoke-mismatch-error.zeek, line 20 and error: type mismatch (M::e and error) error in <...>/function-invoke-mismatch-error.zeek, line 20: argument type mismatch in function call (M::to_string(M::e)) Record coercion also reports noisy errors when coercing to a type that has errors for individual fields, type clashing with "error": $ zeek language/record-field-error.zeek error in ./language/record-coerce-error.zeek, line 8: identifier not defined: MyEnumTypo error in ./language/record-coerce-error.zeek, line 19 and ./language/record-coerce-error.zeek, line 5: type clash for field "e" ((coerce [$e=MY_ENUM_B, $s=test] to MyRecord) and MyEnum) --- src/Expr.cc | 28 +++++++++++++++++-- .../language.event-invoke-mismatch-error/out | 2 ++ .../out | 2 ++ .../Baseline/language.record-coerce-error/out | 2 ++ .../language/event-invoke-mismatch-error.zeek | 21 ++++++++++++++ .../function-invoke-mismatch-error.zeek | 21 ++++++++++++++ .../btest/language/record-coerce-error.zeek | 16 +++++++++++ 7 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 testing/btest/Baseline/language.event-invoke-mismatch-error/out create mode 100644 testing/btest/Baseline/language.function-invoke-mismatch-error/out create mode 100644 testing/btest/Baseline/language.record-coerce-error/out create mode 100644 testing/btest/language/event-invoke-mismatch-error.zeek create mode 100644 testing/btest/language/function-invoke-mismatch-error.zeek create mode 100644 testing/btest/language/record-coerce-error.zeek diff --git a/src/Expr.cc b/src/Expr.cc index 1fb6202b72..39dc91b565 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4101,6 +4101,20 @@ ValPtr ArithCoerceExpr::Fold(Val* v) const return result; } +// Returns true if the record type or any of its fields have an error. +static bool record_type_has_errors(const RecordType* rt) + { + if ( IsErrorType(rt->Tag()) ) + return true; + + if ( rt->NumFields() > 0 ) + for ( const auto* td : *rt->Types() ) + if ( IsErrorType(td->type->Tag()) ) + return true; + + return false; + } + RecordCoerceExpr::RecordCoerceExpr(ExprPtr arg_op, RecordTypePtr r) : UnaryExpr(EXPR_RECORD_COERCE, std::move(arg_op)) { @@ -4120,6 +4134,12 @@ RecordCoerceExpr::RecordCoerceExpr(ExprPtr arg_op, RecordTypePtr r) RecordType* t_r = type->AsRecordType(); RecordType* sub_r = op->GetType()->AsRecordType(); + if ( record_type_has_errors(t_r) || record_type_has_errors(sub_r) ) + { + SetError(); + return; + } + int map_size = t_r->NumFields(); map.resize(map_size, -1); // -1 = field is not mapped @@ -4574,7 +4594,9 @@ CallExpr::CallExpr(ExprPtr arg_func, ListExprPtr arg_args, bool in_hook, bool _i return; } - if ( ! func_type->MatchesIndex(args.get()) ) + if ( record_type_has_errors(func_type->AsFuncType()->Params()->AsRecordType()) ) + SetError(); + else if ( ! func_type->MatchesIndex(args.get()) ) SetError("argument type mismatch in function call"); else { @@ -4943,7 +4965,9 @@ EventExpr::EventExpr(const char* arg_name, ListExprPtr arg_args) return; } - if ( ! func_type->MatchesIndex(args.get()) ) + if ( record_type_has_errors(func_type->AsFuncType()->Params()->AsRecordType()) ) + SetError(); + else if ( ! func_type->MatchesIndex(args.get()) ) SetError("argument type mismatch in event invocation"); else { diff --git a/testing/btest/Baseline/language.event-invoke-mismatch-error/out b/testing/btest/Baseline/language.event-invoke-mismatch-error/out new file mode 100644 index 0000000000..1e59cfc949 --- /dev/null +++ b/testing/btest/Baseline/language.event-invoke-mismatch-error/out @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in <...>/event-invoke-mismatch-error.zeek, line 8: identifier not defined: MyEnumTypo diff --git a/testing/btest/Baseline/language.function-invoke-mismatch-error/out b/testing/btest/Baseline/language.function-invoke-mismatch-error/out new file mode 100644 index 0000000000..e591a842c6 --- /dev/null +++ b/testing/btest/Baseline/language.function-invoke-mismatch-error/out @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in <...>/function-invoke-mismatch-error.zeek, line 8: identifier not defined: MyEnumTypo diff --git a/testing/btest/Baseline/language.record-coerce-error/out b/testing/btest/Baseline/language.record-coerce-error/out new file mode 100644 index 0000000000..879e4eabff --- /dev/null +++ b/testing/btest/Baseline/language.record-coerce-error/out @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error in <...>/record-coerce-error.zeek, line 8: identifier not defined: MyEnumTypo diff --git a/testing/btest/language/event-invoke-mismatch-error.zeek b/testing/btest/language/event-invoke-mismatch-error.zeek new file mode 100644 index 0000000000..27884c7600 --- /dev/null +++ b/testing/btest/language/event-invoke-mismatch-error.zeek @@ -0,0 +1,21 @@ +# @TEST-DOC: Error in event declaration should not report argument mismatches at call site +# @TEST-EXEC-FAIL: zeek -b %INPUT > out 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out +module M; + +export { + type MyEnum: enum { MY_ENUM_A, MY_ENUM_B }; + global my_event: event(e: MyEnumTypo); +} + +event zeek_init() { + event my_event(MY_ENUM_A); +} + +event zeek_done() { + event my_event(MY_ENUM_B); +} + +function helper(e: MyEnum){ + event my_event(e); +} diff --git a/testing/btest/language/function-invoke-mismatch-error.zeek b/testing/btest/language/function-invoke-mismatch-error.zeek new file mode 100644 index 0000000000..3e0147b13c --- /dev/null +++ b/testing/btest/language/function-invoke-mismatch-error.zeek @@ -0,0 +1,21 @@ +# @TEST-DOC: Error in function declaration should not report argument mismatches at call site +# @TEST-EXEC-FAIL: zeek -b %INPUT > out 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out +module M; + +export { + type MyEnum: enum { MY_ENUM_A, MY_ENUM_B }; + global to_string: function(e: MyEnumTypo): string; +} + +event zeek_init() { + M::to_string(MY_ENUM_A); +} + +event zeek_done() { + M::to_string(MY_ENUM_B); +} + +function helper(e: MyEnum): string { + return M::to_string(e); +} diff --git a/testing/btest/language/record-coerce-error.zeek b/testing/btest/language/record-coerce-error.zeek new file mode 100644 index 0000000000..17813fd41a --- /dev/null +++ b/testing/btest/language/record-coerce-error.zeek @@ -0,0 +1,16 @@ +# @TEST-DOC: Error in Record should not show when trying to coerce to it. +# @TEST-EXEC-FAIL: zeek -b %INPUT > out 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out +export { + type MyEnum: enum { MY_ENUM_A, MY_ENUM_B }; + + type MyRecord: record { + e: MyEnumTypo; + s: string; + }; +} + +event zeek_init() { + local r1 = MyRecord($e=MY_ENUM_B, $s="test"); + local r2: MyRecord = [$e=MY_ENUM_B, $s="test"]; +}