Skip to content

Commit

Permalink
Allow to directly access members of anonymous bitfields.
Browse files Browse the repository at this point in the history
This works now:

    type Foo = unit {
      : bitfield(8) {
        x: 0..3;
        y: 4..7;
      };

      on %done {
        print self.x, self.y;
      }
};

Closes #1468.
  • Loading branch information
rsmmr committed Aug 14, 2023
1 parent e55296b commit 0474c22
Show file tree
Hide file tree
Showing 11 changed files with 108 additions and 6 deletions.
6 changes: 3 additions & 3 deletions spicy/toolchain/include/ast/types/unit-items/field.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ class Field : public hilti::NodeBase, public hilti::node::WithDocString, public
std::optional<Expression> repeat, const std::vector<Expression>& sinks,
std::optional<AttributeSet> attrs = {}, std::optional<Expression> cond = {},
const std::vector<Hook>& hooks = {}, Meta m = Meta())
: NodeBase(nodes((id ? id : _uniquer.get("anon")), hilti::type::pruneWalk(std::move(type)), hilti::type::auto_,
hilti::node::none, hilti::type::auto_, node::none, std::move(repeat), std::move(attrs),
std::move(cond), args, sinks, hooks),
: NodeBase(nodes((id ? id : _uniquer.get("__anon")), hilti::type::pruneWalk(std::move(type)),
hilti::type::auto_, hilti::node::none, hilti::type::auto_, node::none, std::move(repeat),
std::move(attrs), std::move(cond), args, sinks, hooks),
std::move(m)),
_is_forwarding(false),
_is_transient(false),
Expand Down
2 changes: 1 addition & 1 deletion spicy/toolchain/src/compiler/codegen/parser-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ struct ProductionVisitor
builder()->addAssign(destination(), builder::default_(field->itemType()));
}

else if ( field->isAnonymous() || field->isSkip() ) {
else if ( (field->isAnonymous() || field->isSkip()) && ! field->itemType().isA<type::Bitfield>() ) {
// We won't have a field to store the value in, create a temporary.
auto dst = builder()->addTmp(fmt("transient_%s", field->id()), field->itemType());
pushDestination(dst);
Expand Down
2 changes: 1 addition & 1 deletion spicy/toolchain/src/compiler/codegen/parsers/types.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ struct Visitor : public hilti::visitor::PreOrder<Expression, Visitor> {
}

result_t operator()(const hilti::type::Bitfield& t, position_t p) {
std::optional<Expression> bitorder = builder::id("spicy::BitOrder::LSB0");
std::optional<Expression> bitorder = builder::id("hilti::BitOrder::LSB0");

if ( auto attrs = t.attributes() ) {
if ( auto a = AttributeSet::find(*attrs, "&bit-order") )
Expand Down
3 changes: 2 additions & 1 deletion spicy/toolchain/src/compiler/codegen/unit-builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ struct FieldBuilder : public hilti::visitor::PreOrder<void, FieldBuilder> {
if ( auto x = AttributeSet::find(f.attributes(), "&default") )
attrs = AttributeSet::add(attrs, *x);

if ( f.isAnonymous() || f.isSkip() || f.parseType().isA<type::Void>() )
if ( (f.isAnonymous() || f.isSkip() || f.parseType().isA<type::Void>()) &&
! f.itemType().isA<type::Bitfield>() )
// This field will never make it into the C++ struct. We still
// carry it around though as that makes type inference easier at
// times, and also can improve error messages.
Expand Down
42 changes: 42 additions & 0 deletions spicy/toolchain/src/compiler/visitors/normalizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,48 @@ struct Visitor : public hilti::visitor::PostOrder<void, Visitor> {
}
}

void operator()(const operator_::unit::MemberNonConst& o, position_t p) {
auto unit = o.op0().type().tryAs<type::Unit>();
auto id = o.op1().tryAs<hilti::expression::Member>()->id();
if ( unit && id && ! unit->itemByName(id) ) {
// See if we we got an anonymous bitfield with a member of that
// name. If so, rewrite the access to transparently to refer to the
// member through the field's internal name.
bool found_field = false;
for ( const auto& f : unit->items<type::unit::item::Field>() ) {
if ( ! f.isAnonymous() )
continue;

auto t = f.itemType().tryAs<type::Bitfield>();
if ( ! t )
continue;

auto bits = t->bits(id);
if ( ! bits )
continue;

if ( found_field ) {
hilti::logger().error(
hilti::util::fmt("unit '%s' has multiple anonymous bitfields with member '%s'", *unit->id(),
id));
return;
}

auto access_field =
operator_::unit::MemberNonConst::Operator().instantiate({o.op0(),
hilti::expression::Member(f.id())},
o.meta());
auto access_bits =
bitfield::Member::Operator().instantiate({std::move(access_field), o.op1()}, o.meta());

logChange(p.node, access_bits);
p.node = access_bits;
modified = true;
found_field = true;
}
}
}

void operator()(const type::Unit& u, position_t p) {
if ( ! p.node.as<Type>().typeID() )
return;
Expand Down
9 changes: 9 additions & 0 deletions spicy/toolchain/src/compiler/visitors/validator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -718,6 +718,15 @@ struct VisitorPost : public hilti::visitor::PreOrder<void, VisitorPost>, public
error(fmt("'%s' can be used at most once", a), p);
}
}

// For anonymous bitfields, make sure we don't introduce a name clash
// when mapping their fields into the unit space.
if ( auto bf = f.parseType().tryAs<type::Bitfield>(); bf && f.isAnonymous() ) {
for ( const auto& b : bf->bits() ) {
if ( auto field = p.findParent<type::Unit>()->get().itemByName(b.id()); field )
error(fmt("field '%s' of anonymous bitfield clashes with other unit field", b.id()), p);
}
}
}

void operator()(const spicy::type::unit::item::UnresolvedField& u, position_t p) {
Expand Down
3 changes: 3 additions & 0 deletions tests/Baseline/spicy.types.bitfield.anonymous-fail-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] unit 'Test::Bar' has multiple anonymous bitfields with member 'x'
[error] spicyc: errors encountered during normalizing
3 changes: 3 additions & 0 deletions tests/Baseline/spicy.types.bitfield.anonymous-fail/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] <...>/anonymous-fail.spicy:8:12-11:5: field 'x' of anonymous bitfield clashes with other unit field
[error] spicyc: aborting after errors
2 changes: 2 additions & 0 deletions tests/Baseline/spicy.types.bitfield.anonymous-field/output
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
15, 15
27 changes: 27 additions & 0 deletions tests/spicy/types/bitfield/anonymous-fail.spicy
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# @TEST-EXEC-FAIL: spicyc -c %INPUT >output 2>&1
# @TEST-EXEC: btest-diff output
# @TEST-DOC: Check thqt an anonymous bitfield's field name cannot shadow other fields.

module Test;

type Foo = unit {
x: uint8;
: bitfield(8) {
x: 0..3;
};
};

# @TEST-START-NEXT

module Test;

type Bar = unit {
: bitfield(8) {
x: 0..3;
};
: bitfield(8) {
x: 0..3;
};

on %done { print self.x; }
};
15 changes: 15 additions & 0 deletions tests/spicy/types/bitfield/anonymous-field.spicy
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# @TEST-EXEC: printf '\377' | spicy-driver %INPUT >output
# @TEST-EXEC: btest-diff output

module Test;

public type Foo = unit {
: bitfield(8) {
x: 0..3;
y: 4..7;
};

on %done {
print self.x, self.y;
}
};

0 comments on commit 0474c22

Please sign in to comment.