From 0474c2264fa99d40b6954bcc779f1179934653c2 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Fri, 4 Aug 2023 14:34:51 +0200 Subject: [PATCH] Allow to directly access members of anonymous bitfields. This works now: type Foo = unit { : bitfield(8) { x: 0..3; y: 4..7; }; on %done { print self.x, self.y; } }; Closes #1468. --- .../include/ast/types/unit-items/field.h | 6 +-- .../src/compiler/codegen/parser-builder.cc | 2 +- .../src/compiler/codegen/parsers/types.cc | 2 +- .../src/compiler/codegen/unit-builder.cc | 3 +- .../src/compiler/visitors/normalizer.cc | 42 +++++++++++++++++++ .../src/compiler/visitors/validator.cc | 9 ++++ .../output | 3 ++ .../output | 3 ++ .../output | 2 + .../spicy/types/bitfield/anonymous-fail.spicy | 27 ++++++++++++ .../types/bitfield/anonymous-field.spicy | 15 +++++++ 11 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 tests/Baseline/spicy.types.bitfield.anonymous-fail-2/output create mode 100644 tests/Baseline/spicy.types.bitfield.anonymous-fail/output create mode 100644 tests/Baseline/spicy.types.bitfield.anonymous-field/output create mode 100644 tests/spicy/types/bitfield/anonymous-fail.spicy create mode 100644 tests/spicy/types/bitfield/anonymous-field.spicy diff --git a/spicy/toolchain/include/ast/types/unit-items/field.h b/spicy/toolchain/include/ast/types/unit-items/field.h index 3992f971c..5c2258dea 100644 --- a/spicy/toolchain/include/ast/types/unit-items/field.h +++ b/spicy/toolchain/include/ast/types/unit-items/field.h @@ -25,9 +25,9 @@ class Field : public hilti::NodeBase, public hilti::node::WithDocString, public std::optional repeat, const std::vector& sinks, std::optional attrs = {}, std::optional cond = {}, const std::vector& 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), diff --git a/spicy/toolchain/src/compiler/codegen/parser-builder.cc b/spicy/toolchain/src/compiler/codegen/parser-builder.cc index e964b241d..7bd9d99b9 100644 --- a/spicy/toolchain/src/compiler/codegen/parser-builder.cc +++ b/spicy/toolchain/src/compiler/codegen/parser-builder.cc @@ -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() ) { // 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); diff --git a/spicy/toolchain/src/compiler/codegen/parsers/types.cc b/spicy/toolchain/src/compiler/codegen/parsers/types.cc index 41dbd806a..23c1e99df 100644 --- a/spicy/toolchain/src/compiler/codegen/parsers/types.cc +++ b/spicy/toolchain/src/compiler/codegen/parsers/types.cc @@ -117,7 +117,7 @@ struct Visitor : public hilti::visitor::PreOrder { } result_t operator()(const hilti::type::Bitfield& t, position_t p) { - std::optional bitorder = builder::id("spicy::BitOrder::LSB0"); + std::optional bitorder = builder::id("hilti::BitOrder::LSB0"); if ( auto attrs = t.attributes() ) { if ( auto a = AttributeSet::find(*attrs, "&bit-order") ) diff --git a/spicy/toolchain/src/compiler/codegen/unit-builder.cc b/spicy/toolchain/src/compiler/codegen/unit-builder.cc index ec269206e..581ceadad 100644 --- a/spicy/toolchain/src/compiler/codegen/unit-builder.cc +++ b/spicy/toolchain/src/compiler/codegen/unit-builder.cc @@ -41,7 +41,8 @@ struct FieldBuilder : public hilti::visitor::PreOrder { if ( auto x = AttributeSet::find(f.attributes(), "&default") ) attrs = AttributeSet::add(attrs, *x); - if ( f.isAnonymous() || f.isSkip() || f.parseType().isA() ) + if ( (f.isAnonymous() || f.isSkip() || f.parseType().isA()) && + ! f.itemType().isA() ) // 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. diff --git a/spicy/toolchain/src/compiler/visitors/normalizer.cc b/spicy/toolchain/src/compiler/visitors/normalizer.cc index 25825d5b2..856ab7319 100644 --- a/spicy/toolchain/src/compiler/visitors/normalizer.cc +++ b/spicy/toolchain/src/compiler/visitors/normalizer.cc @@ -227,6 +227,48 @@ struct Visitor : public hilti::visitor::PostOrder { } } + void operator()(const operator_::unit::MemberNonConst& o, position_t p) { + auto unit = o.op0().type().tryAs(); + auto id = o.op1().tryAs()->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() ) { + if ( ! f.isAnonymous() ) + continue; + + auto t = f.itemType().tryAs(); + 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().typeID() ) return; diff --git a/spicy/toolchain/src/compiler/visitors/validator.cc b/spicy/toolchain/src/compiler/visitors/validator.cc index b9939ea3a..cc8f8e7a9 100644 --- a/spicy/toolchain/src/compiler/visitors/validator.cc +++ b/spicy/toolchain/src/compiler/visitors/validator.cc @@ -718,6 +718,15 @@ struct VisitorPost : public hilti::visitor::PreOrder, 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(); bf && f.isAnonymous() ) { + for ( const auto& b : bf->bits() ) { + if ( auto field = p.findParent()->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) { diff --git a/tests/Baseline/spicy.types.bitfield.anonymous-fail-2/output b/tests/Baseline/spicy.types.bitfield.anonymous-fail-2/output new file mode 100644 index 000000000..08f2339e0 --- /dev/null +++ b/tests/Baseline/spicy.types.bitfield.anonymous-fail-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] unit 'Test::Bar' has multiple anonymous bitfields with member 'x' +[error] spicyc: errors encountered during normalizing diff --git a/tests/Baseline/spicy.types.bitfield.anonymous-fail/output b/tests/Baseline/spicy.types.bitfield.anonymous-fail/output new file mode 100644 index 000000000..5e7f7292c --- /dev/null +++ b/tests/Baseline/spicy.types.bitfield.anonymous-fail/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] <...>/anonymous-fail.spicy:8:12-11:5: field 'x' of anonymous bitfield clashes with other unit field +[error] spicyc: aborting after errors diff --git a/tests/Baseline/spicy.types.bitfield.anonymous-field/output b/tests/Baseline/spicy.types.bitfield.anonymous-field/output new file mode 100644 index 000000000..29e274b01 --- /dev/null +++ b/tests/Baseline/spicy.types.bitfield.anonymous-field/output @@ -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 diff --git a/tests/spicy/types/bitfield/anonymous-fail.spicy b/tests/spicy/types/bitfield/anonymous-fail.spicy new file mode 100644 index 000000000..153b9cc13 --- /dev/null +++ b/tests/spicy/types/bitfield/anonymous-fail.spicy @@ -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; } +}; diff --git a/tests/spicy/types/bitfield/anonymous-field.spicy b/tests/spicy/types/bitfield/anonymous-field.spicy new file mode 100644 index 000000000..ef0f8b309 --- /dev/null +++ b/tests/spicy/types/bitfield/anonymous-field.spicy @@ -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; + } +};