Unify type comparisions for records.

For records, same_type(r1, r2) would not check if the fields'
attributes match as well. That seems like an oversight, and some
callers of same_type() did indeed add that check on their end. This
commit moves the check into same_type() itself. That generally doesn't
seem make any differences except for a couple of places validating
code, which we update a bit. That in turn leans to slightly different
(better?) error messages for a couple of test cases.
This commit is contained in:
Robin Sommer 2020-10-04 16:42:44 +00:00
parent a2577891e0
commit d782c60f19
5 changed files with 45 additions and 52 deletions

View file

@ -2168,22 +2168,14 @@ bool AssignExpr::TypeCheck(const AttributesPtr& attrs)
if ( op1->GetType()->IsSet() ) if ( op1->GetType()->IsSet() )
op2 = make_intrusive<SetConstructorExpr>( op2 = make_intrusive<SetConstructorExpr>(
cast_intrusive<ListExpr>(op2), std::move(attr_copy)); cast_intrusive<ListExpr>(op2), std::move(attr_copy), op1->GetType());
else else
op2 = make_intrusive<TableConstructorExpr>( op2 = make_intrusive<TableConstructorExpr>(
cast_intrusive<ListExpr>(op2), std::move(attr_copy)); cast_intrusive<ListExpr>(op2), std::move(attr_copy), op1->GetType());
if ( ! empty_list_assignment && ! same_type(op1->GetType(), op2->GetType()) ) // The constructor expressions are performing the type
{ // checking and will set op2 to an error state on failure.
if ( op1->GetType()->IsSet() ) return ! op2->IsError();
ExprError("set type mismatch in assignment");
else
ExprError("table type mismatch in assignment");
return false;
}
return true;
} }
if ( bt1 == TYPE_VECTOR ) if ( bt1 == TYPE_VECTOR )
@ -2207,21 +2199,7 @@ bool AssignExpr::TypeCheck(const AttributesPtr& attrs)
op2->GetType()->Tag() == TYPE_RECORD ) op2->GetType()->Tag() == TYPE_RECORD )
{ {
if ( same_type(op1->GetType(), op2->GetType()) ) if ( same_type(op1->GetType(), op2->GetType()) )
{
RecordType* rt1 = op1->GetType()->AsRecordType();
RecordType* rt2 = op2->GetType()->AsRecordType();
// Make sure the attributes match as well.
for ( int i = 0; i < rt1->NumFields(); ++i )
{
const TypeDecl* td1 = rt1->FieldDecl(i);
const TypeDecl* td2 = rt2->FieldDecl(i);
if ( same_attrs(td1->attrs.get(), td2->attrs.get()) )
// Everything matches.
return true; return true;
}
}
// Need to coerce. // Need to coerce.
op2 = make_intrusive<RecordCoerceExpr>(std::move(op2), op1->GetType<RecordType>()); op2 = make_intrusive<RecordCoerceExpr>(std::move(op2), op1->GetType<RecordType>());
@ -3195,17 +3173,18 @@ TableConstructorExpr::TableConstructorExpr(ListExprPtr constructor_list,
const auto& indices = type->AsTableType()->GetIndices()->GetTypes(); const auto& indices = type->AsTableType()->GetIndices()->GetTypes();
const ExprPList& cle = op->AsListExpr()->Exprs(); const ExprPList& cle = op->AsListExpr()->Exprs();
// check and promote all index expressions in ctor list // check and promote all assign expressions in ctor list
for ( const auto& expr : cle ) for ( const auto& expr : cle )
{ {
if ( expr->Tag() != EXPR_ASSIGN ) if ( expr->Tag() != EXPR_ASSIGN )
continue; continue;
Expr* idx_expr = expr->AsAssignExpr()->Op1(); auto idx_expr = expr->AsAssignExpr()->Op1();
auto val_expr = expr->AsAssignExpr()->Op2();
if ( idx_expr->Tag() != EXPR_LIST ) auto yield_type = GetType()->AsTableType()->Yield().get();
continue;
// Promote LHS
assert(idx_expr->Tag() == EXPR_LIST);
ExprPList& idx_exprs = idx_expr->AsListExpr()->Exprs(); ExprPList& idx_exprs = idx_expr->AsListExpr()->Exprs();
if ( idx_exprs.length() != static_cast<int>(indices.size()) ) if ( idx_exprs.length() != static_cast<int>(indices.size()) )
@ -3229,6 +3208,19 @@ TableConstructorExpr::TableConstructorExpr(ListExprPtr constructor_list,
} }
ExprError("inconsistent types in table constructor"); ExprError("inconsistent types in table constructor");
return;
}
// Promote RHS
if ( auto promoted_val = check_and_promote_expr(val_expr, yield_type) )
{
if ( promoted_val != val_expr )
expr->AsAssignExpr()->SetOp2(promoted_val);
}
else
{
ExprError("inconsistent types in table constructor");
return;
} }
} }
} }
@ -3328,8 +3320,8 @@ SetConstructorExpr::SetConstructorExpr(ListExprPtr constructor_list,
Expr* ce = cle[i]; Expr* ce = cle[i];
ListExpr* le = ce->AsListExpr(); ListExpr* le = ce->AsListExpr();
if ( ce->Tag() == EXPR_LIST && assert(ce->Tag() == EXPR_LIST);
check_and_promote_exprs(le, type->AsTableType()->GetIndices().get()) ) if ( check_and_promote_exprs(le, type->AsTableType()->GetIndices().get()) )
{ {
if ( le != cle[i] ) if ( le != cle[i] )
cle.replace(i, le); cle.replace(i, le);
@ -4985,18 +4977,7 @@ ExprPtr check_and_promote_expr(Expr* const e, zeek::Type* t)
RecordType* et_r = et->AsRecordType(); RecordType* et_r = et->AsRecordType();
if ( same_type(t, et) ) if ( same_type(t, et) )
{
// Make sure the attributes match as well.
for ( int i = 0; i < t_r->NumFields(); ++i )
{
const TypeDecl* td1 = t_r->FieldDecl(i);
const TypeDecl* td2 = et_r->FieldDecl(i);
if ( same_attrs(td1->attrs.get(), td2->attrs.get()) )
// Everything matches perfectly.
return {NewRef{}, e}; return {NewRef{}, e};
}
}
if ( record_promotion_compatible(t_r, et_r) ) if ( record_promotion_compatible(t_r, et_r) )
return make_intrusive<RecordCoerceExpr>( return make_intrusive<RecordCoerceExpr>(

View file

@ -504,6 +504,11 @@ public:
ValPtr InitVal(const zeek::Type* t, ValPtr aggr) const override; ValPtr InitVal(const zeek::Type* t, ValPtr aggr) const override;
bool IsPure() const override; bool IsPure() const override;
void SetOp2(ExprPtr e)
{
op2 = e;
}
protected: protected:
bool TypeCheck(const AttributesPtr& attrs = nullptr); bool TypeCheck(const AttributesPtr& attrs = nullptr);
bool TypeCheckArithmetics(TypeTag bt1, TypeTag bt2); bool TypeCheckArithmetics(TypeTag bt1, TypeTag bt2);

View file

@ -1690,6 +1690,9 @@ bool same_type(const Type& arg_t1, const Type& arg_t2,
if ( (match_record_field_names && ! util::streq(td1->id, td2->id)) || if ( (match_record_field_names && ! util::streq(td1->id, td2->id)) ||
! same_type(td1->type, td2->type, is_init, match_record_field_names) ) ! same_type(td1->type, td2->type, is_init, match_record_field_names) )
return false; return false;
if ( ! same_attrs(td1->attrs.get(), td2->attrs.get()) )
return false;
} }
return true; return true;

View file

@ -1,2 +1,2 @@
error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.record-bad-ctor/record-bad-ctor.zeek, line 6: no type given (asdfasdf) error in /Users/robin/bro/topic/testing/btest/.tmp/language.record-bad-ctor/record-bad-ctor.zeek, line 6: no type given (asdfasdf)
expression error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.record-bad-ctor/record-bad-ctor.zeek, line 7: uninitialized list value ($ports=asdfasdf) error in /Users/robin/bro/topic/testing/btest/.tmp/language.record-bad-ctor/record-bad-ctor.zeek, line 7: non-optional field "ports" missing in initialization ([ports=<uninitialized>])

View file

@ -1,3 +1,7 @@
error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, lines 15-20: table type mismatch in assignment (service_table_bad_yield = table(www, 80 = Internal Web Server, dns1, 53 = Internal DNS 1, dns2, 53 = Internal DNS 2, dhcp-for-wifi, 443 = DHCP Management interface for WiFi)) error in count and /Users/robin/bro/topic/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, line 16: arithmetic mixed with non-arithmetic (count and Internal Web Server)
error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, lines 23-28: table type mismatch in assignment (service_table_bad_index = table(www, 80 = Internal Web Server, dns1, 53 = Internal DNS 1, dns2, 53 = Internal DNS 2, dhcp-for-wifi, 443 = DHCP Management interface for WiFi)) error in /Users/robin/bro/topic/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, lines 15-20: inconsistent types in table constructor (table(www, 80 = Internal Web Server, dns1, 53 = Internal DNS 1, dns2, 53 = Internal DNS 2, dhcp-for-wifi, 443 = DHCP Management interface for WiFi))
error in /home/jon/pro/zeek/zeek/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, line 31: set type mismatch in assignment (test_set_bad = set(1, 2, 3)) error in count and /Users/robin/bro/topic/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, line 24: arithmetic mixed with non-arithmetic (count and 80)
error in /Users/robin/bro/topic/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, lines 23-28: inconsistent types in table constructor (table(www, 80 = Internal Web Server, dns1, 53 = Internal DNS 1, dns2, 53 = Internal DNS 2, dhcp-for-wifi, 443 = DHCP Management interface for WiFi))
error in string and /Users/robin/bro/topic/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, line 31: arithmetic mixed with non-arithmetic (string and 1)
error in /Users/robin/bro/topic/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, line 31 and string: type mismatch (1 and string)
error in /Users/robin/bro/topic/testing/btest/.tmp/language.table-list-assign-type-check/table-list-assign-type-check.zeek, line 31: inconsistent type in set constructor (set(1, 2, 3))