From e529e9b1e579dde6b63a9182111d9eb988e066a7 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Tue, 15 Oct 2024 17:03:37 +0200 Subject: [PATCH 1/2] Provide proper error message when trying access an unknown unit field. Returning `type::Unknown` instead of `type::Auto` will let the resolver process stop, allowing an already existing error message to kick in later. We were doing it this way already for structs, but not for units. This also includes a fix for a bug with finding bitfield ranges by ID, which was triggered by the change. Closes #1790. --- spicy/toolchain/src/ast/operators/unit.cc | 2 +- spicy/toolchain/src/ast/types/unit.cc | 2 +- .../spicy.types.unit.unknown-field/output | 3 +++ tests/spicy/types/unit/unknown-field.spicy | 15 +++++++++++++++ 4 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 tests/Baseline/spicy.types.unit.unknown-field/output create mode 100644 tests/spicy/types/unit/unknown-field.spicy diff --git a/spicy/toolchain/src/ast/operators/unit.cc b/spicy/toolchain/src/ast/operators/unit.cc index 37da54aeb..a2ec01f76 100644 --- a/spicy/toolchain/src/ast/operators/unit.cc +++ b/spicy/toolchain/src/ast/operators/unit.cc @@ -31,7 +31,7 @@ QualifiedType* itemType(hilti::Builder* builder, const Expressions& operands) { else if ( auto bitrange = unit->findRangeInAnonymousBitField(id).second ) return bitrange->itemType(); else - return builder->qualifiedType(builder->typeAuto(), hilti::Constness::Const); + return builder->qualifiedType(builder->typeUnknown(), hilti::Constness::Const); } QualifiedType* contextResult(hilti::Builder* builder, const Expressions& operands, hilti::Constness constness) { diff --git a/spicy/toolchain/src/ast/types/unit.cc b/spicy/toolchain/src/ast/types/unit.cc index 77d1c8d03..265c282af 100644 --- a/spicy/toolchain/src/ast/types/unit.cc +++ b/spicy/toolchain/src/ast/types/unit.cc @@ -97,7 +97,7 @@ static std::pair findRange if ( ! field->isAnonymous() ) continue; - auto t = field->itemType()->type()->tryAs(); + auto t = field->originalType()->type()->tryAs(); if ( ! t ) continue; diff --git a/tests/Baseline/spicy.types.unit.unknown-field/output b/tests/Baseline/spicy.types.unit.unknown-field/output new file mode 100644 index 000000000..7c0390639 --- /dev/null +++ b/tests/Baseline/spicy.types.unit.unknown-field/output @@ -0,0 +1,3 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +[error] <...>/unknown-field.spicy:13:14-13:22: unit does not have field 'bar_' +[error] spicyc: aborting after errors diff --git a/tests/spicy/types/unit/unknown-field.spicy b/tests/spicy/types/unit/unknown-field.spicy new file mode 100644 index 000000000..80af38d88 --- /dev/null +++ b/tests/spicy/types/unit/unknown-field.spicy @@ -0,0 +1,15 @@ +# @TEST-EXEC-FAIL: spicyc -p %INPUT >output 2>&1 +# @TEST-EXEC: btest-diff output +# +# @TEST-DOC: Check that we get a proper error message when trying to access an unknown unit field; regression test for #1790 + +module Test; + +type Msg = unit { + var x: uint16; + bar: uint16; + + on %done { + self.x = self.bar_; + } +}; From cbd269ae10576a846c6ab2aa6dc707bf5607b79b Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Wed, 16 Oct 2024 14:14:31 +0200 Subject: [PATCH 2/2] Prioritize error message reporting unknown field. This suppresses some non-interesting follow-up errors. Closes #1792. --- spicy/toolchain/src/ast/operators/unit.cc | 2 +- .../spicy.types.unit.unknown-field-2/output | 3 +++ tests/spicy/types/unit/unknown-field.spicy | 19 ++++++++++++++++++- 3 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 tests/Baseline/spicy.types.unit.unknown-field-2/output diff --git a/spicy/toolchain/src/ast/operators/unit.cc b/spicy/toolchain/src/ast/operators/unit.cc index a2ec01f76..fddbf5b1f 100644 --- a/spicy/toolchain/src/ast/operators/unit.cc +++ b/spicy/toolchain/src/ast/operators/unit.cc @@ -18,7 +18,7 @@ void checkName(hilti::expression::ResolvedOperator* op) { auto i = op->op0()->type()->type()->as()->itemByName(id); if ( ! i ) - op->addError(hilti::util::fmt("unit does not have field '%s'", id)); + op->addError(hilti::util::fmt("unit does not have field '%s'", id), node::ErrorPriority::High); } diff --git a/tests/Baseline/spicy.types.unit.unknown-field-2/output b/tests/Baseline/spicy.types.unit.unknown-field-2/output new file mode 100644 index 000000000..a24d2ab59 --- /dev/null +++ b/tests/Baseline/spicy.types.unit.unknown-field-2/output @@ -0,0 +1,3 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +[error] <...>/unknown-field.spicy:12:16-12:23: unit does not have field 'tss' +[error] spicyc: aborting after errors diff --git a/tests/spicy/types/unit/unknown-field.spicy b/tests/spicy/types/unit/unknown-field.spicy index 80af38d88..3989c44d1 100644 --- a/tests/spicy/types/unit/unknown-field.spicy +++ b/tests/spicy/types/unit/unknown-field.spicy @@ -1,7 +1,7 @@ # @TEST-EXEC-FAIL: spicyc -p %INPUT >output 2>&1 # @TEST-EXEC: btest-diff output # -# @TEST-DOC: Check that we get a proper error message when trying to access an unknown unit field; regression test for #1790 +# @TEST-DOC: Check that we get a proper error message when trying to access an unknown unit field; regression test for #1790 and #1792 module Test; @@ -13,3 +13,20 @@ type Msg = unit { self.x = self.bar_; } }; + +# @TEST-START-NEXT + +module Test; + +type T = unit() { + x: uint8; +}; + +public type U = unit() { + len: uint32; + + ts: T[] &size=self.len { + for ( e in self.tss ) + print e.x; + } +};