Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/topic/robin/gh-1664-bitrange-con…
Browse files Browse the repository at this point in the history
…vert'

* origin/topic/robin/gh-1664-bitrange-convert:
  Fix `&convert` typing issue with bit ranges.
  Suppress new clang-tidy warnings.
  • Loading branch information
rsmmr committed May 6, 2024
2 parents 01eae54 + 14437ab commit 92f7654
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 7 deletions.
2 changes: 2 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ Checks: '
-readability-qualified-auto,
-readability-simplify-boolean-expr,
-readability-use-anyofallof,
-readability-identifier-length,
-readability-implicit-bool-conversion,
'

HeaderFilterRegex: '(hilti|spicy)/[a-zA-Z]+/include/([a-zA-Z]+/)*[a-zA-Z]+\.h'
Expand Down
22 changes: 22 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
@@ -1,3 +1,25 @@
1.11.0-dev.142 | 2024-05-06 15:14:40 +0200

* GH-1664: Fix `&convert` typing issue with bit ranges. (Robin Sommer, Corelight)

Turns out #1664 was only indirectly related to the `&convert` itself;
the real issue was that we couldn't assign one bitfield struct to
another if their field types didn't match exactly, even in cases where
at the C++ level there was no meaningful difference. In this case we
ended up with a field that had a C++ type `rt::Bool` in one type and
`bool` in another, leading to errors when assigning the latter to the
former. We now allow to creating instances of the former from the
latter through standard C++ type conversions on a per field basis.

* Suppress new `clang-tidy` warnings. (Robin Sommer, Corelight)

* Fix a Spicy scoping issue across imports. (Robin Sommer)

We could get a bogus "unknown ID" error for default arguments of
functions defined in an imported module if that default argument was
itself referring to an identifier inside yet another imported module.
The test case shows the exact situation that was broken.

1.11.0-dev.137 | 2024-04-25 13:38:42 +0200

* Remove Spicy parser support for unsupported `&priority` attribute. (Benjamin Bannier, Corelight)
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.11.0-dev.137
1.11.0-dev.142
15 changes: 13 additions & 2 deletions hilti/runtime/include/types/bitfield.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,26 @@ struct isBitfield {};
/// printing.
template<typename... Ts>
struct Bitfield : public trait::isBitfield {
std::tuple<std::optional<Ts>...> value;
using value_type = std::tuple<std::optional<Ts>...>;

Bitfield(value_type v = {}) : value(std::move(v)) {}

/**
* Support instantiation from another bitfield type as long as all types
* convert over.
*/
template<typename... Us>
Bitfield(Bitfield<Us...> other) : value(std::move(other.value)) {}

value_type value;
};

/// Construct a Bitfield.
///
/// Since a bitfield always owns its values this takes all fields by value.
template<typename... Ts>
Bitfield<Ts...> make_bitfield(Ts... args) {
return Bitfield<Ts...>{{}, std::make_tuple(std::move(args)...)};
return {typename Bitfield<Ts...>::value_type(std::move(args)...)};
}

namespace bitfield {
Expand Down
3 changes: 1 addition & 2 deletions hilti/toolchain/src/compiler/codegen/ctors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ struct Visitor : hilti::visitor::PreOrder {
values.emplace_back("std::nullopt");
}

result =
fmt("hilti::rt::Bitfield<%s>{{}, std::make_tuple(%s)}", util::join(types, ", "), util::join(values, ", "));
result = fmt("hilti::rt::Bitfield<%s>(std::make_tuple(%s))", util::join(types, ", "), util::join(values, ", "));
}
void operator()(ctor::Bool* n) final { result = fmt("::hilti::rt::Bool(%s)", n->value() ? "true" : "false"); }

Expand Down
3 changes: 3 additions & 0 deletions tests/Baseline/spicy.types.bitfield.parse-and-convert/output
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,7 @@ mini::Y {
d: {
d: foo
}
e: {
d: True
}
}
4 changes: 4 additions & 0 deletions tests/hilti/types/bitfield/convert.hlt
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ import hilti;
type X = bitfield(32) {
a: 0..4 &convert="string";
b: 1..2 &convert=(2 * $$);
c: 3..4 &convert=($$ == 2);
d: 5..6;
};

global X x = 255;
assert x.a == "string";
assert x.b == 6;
#assert x.c == True;
assert x.d == 3;

}
8 changes: 6 additions & 2 deletions tests/spicy/types/bitfield/parse-and-convert.spicy
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# @TEST-EXEC: spicyc -dj %INPUT -o mini.hlto >>output
# @TEST-EXEC: printf '\377\377\377\377' | spicy-dump -p mini::X mini.hlto >>output
# @TEST-EXEC: printf '\377\377\377\377' | spicy-dump -p mini::Y mini.hlto >>output
# @TEST-EXEC: printf '\377\377\377\377\377' | spicy-dump -p mini::X mini.hlto >>output
# @TEST-EXEC: printf '\377\377\377\377\377' | spicy-dump -p mini::Y mini.hlto >>output
# @TEST-EXEC: btest-diff output

module mini;
Expand Down Expand Up @@ -30,6 +30,10 @@ public type Y = unit {
d: bitfield(8) {
d: 0..7 &convert="foo";
};

e: bitfield(8) {
d: 0..7 &convert=($$ == 255); # regression test for #1664
};
};

function FOO(): string { return "foo"; }
Expand Down

0 comments on commit 92f7654

Please sign in to comment.