Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/topic/robin/gh-1790-unknown-field'
Browse files Browse the repository at this point in the history
* origin/topic/robin/gh-1790-unknown-field:
  Prioritize error message reporting unknown field.
  Provide proper error message when trying access an unknown unit field.
  • Loading branch information
rsmmr committed Oct 17, 2024
2 parents 156d11f + cbd269a commit 0e3bc48
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 4 deletions.
39 changes: 39 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,3 +1,42 @@
1.12.0-dev.150 | 2024-10-17 10:16:57 +0200

* GH-1792: Prioritize error message reporting unknown field. (Robin Sommer, Corelight)

This suppresses some non-interesting follow-up errors.

* GH-1790: Provide proper error message when trying access an unknown unit field. (Robin Sommer, Corelight)

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.

* GH-1791: Fix usage of `&convert` with unit's requiring parameters. (Robin Sommer, Corelight)

The generated code needs to create a temporary instance of the type,
but doesn't have any arguments to provide to it. But that's ok, and we
now let the validation pass and just instantiate a default-constructed
instance.

However, this change now requires an additional validator check on the
Spicy side to ensure fields giving arguments to types do so correctly
Before we happened to check that implicitly on the HILTI-side through
code that now would let it pass if no arguments were given.

* Factor out logic to validate arguments given to a type. (Robin Sommer, Corelight)

This will allow Spicy's validator to use it as well.

We also add two options to (1) accept usages where no argument is given at
all even when the type would normally require it,;and (2) skip any
actual type checking, and just confirm argument count. In a subsequent
change, we'll use (1) to fall-back to a type's default constructor,
and (2) to check type usage inside unit fields where we haven't
fully coerced arguments yet.

1.12.0-dev.144 | 2024-10-16 16:10:40 +0200

* GH-1861: Disallow ignored attributes on type aliases. (Evan Typanski, Corelight)
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.12.0-dev.144
1.12.0-dev.150
4 changes: 2 additions & 2 deletions spicy/toolchain/src/ast/operators/unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ void checkName(hilti::expression::ResolvedOperator* op) {
auto i = op->op0()->type()->type()->as<type::Unit>()->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);
}


Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion spicy/toolchain/src/ast/types/unit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ static std::pair<unit::item::Field*, hilti::type::bitfield::BitRange*> findRange
if ( ! field->isAnonymous() )
continue;

auto t = field->itemType()->type()->tryAs<hilti::type::Bitfield>();
auto t = field->originalType()->type()->tryAs<hilti::type::Bitfield>();
if ( ! t )
continue;

Expand Down
3 changes: 3 additions & 0 deletions tests/Baseline/spicy.types.unit.unknown-field-2/output
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions tests/Baseline/spicy.types.unit.unknown-field/output
Original file line number Diff line number Diff line change
@@ -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
32 changes: 32 additions & 0 deletions tests/spicy/types/unit/unknown-field.spicy
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# @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 and #1792

module Test;

type Msg = unit {
var x: uint16;
bar: uint16;

on %done {
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;
}
};

0 comments on commit 0e3bc48

Please sign in to comment.