Merge remote-tracking branch 'origin/topic/robin/bitfield-improvements'

* origin/topic/robin/bitfield-improvements:
  [spicy-dump] Level up anonymous bitfields.
  [spicy-dump] Make offsets in JSON output useful.
  [spicy-dump] Change format how bitfields are printed.
  Include a flag into a unit field's type information indicating if it's anonymous.
  Allow to directly access members of anonymous bitfields.
  Switch Spicy over to use the new HILTI-side bitfield type.
  Implement remaining pieces to make `bitfield` a full HILTI type.
  Move `bitfield` type from Spicy to HILTI.
  Fix comparison for resolved operators.

hilti/runtime/include/result.h

@@ -185,4 +185,10 @@ class Result {
     std::variant<T, result::Error> _value;
 };
 
+/** Similar to `std::make_optional`, construct a result from a value. */
+template<typename T>
+Result<T> make_result(T&& t) {
+    return Result<T>(std::forward<T>(t));
+}
+
 } // namespace hilti::rt

hilti/runtime/include/type-info.h

@@ -378,6 +378,68 @@ class Address : public detail::AtomicType<hilti::rt::Address> {};
 /** Type information for type ``any`. */
 class Any : public detail::ValueLessType {};
 
+class Bitfield;
+
+namespace bitfield {
+
+/** Auxiliary type information for type ``bitfield`` describing one of its fields. */
+class Bits {
+public:
+    /**
+     * Constructor.
+     *
+     * @param name ID of the field
+     * @param type type of the field
+     * @param offset offset of the field inside the bitfield's storage tuple.
+     */
+    Bits(const char* name, const TypeInfo* type, std::ptrdiff_t offset) : name(name), type(type), offset(offset) {}
+
+    const std::string name; /**< ID of the field, with an empty string indicating no name */
+    const TypeInfo* type;   /**< type of the field */
+
+private:
+    friend class type_info::Bitfield;
+
+    const std::ptrdiff_t offset;
+};
+
+}; // namespace bitfield
+
+/** Auxiliary type information for type ``bitfield`. */
+class Bitfield {
+public:
+    /**
+     * Constructor
+     *
+     * @param labels the bitfield's fields
+     */
+    Bitfield(std::vector<bitfield::Bits> bits) : _bits(std::move(bits)) {}
+
+    /** Returns the bitfield's fields. */
+    const auto& bits() const { return _bits; }
+
+    /**
+     * Returns a vector that can be iterated over to visit all the fields.
+     *
+     * @param v the value referring to the bitfield to iterate over
+     *
+     * @return a vector of pairs ``(field, value)`` where *field* is the
+     * current ``bitfield::Bit` and *value* is the field's value.
+     */
+    auto iterate(const Value& v) const {
+        std::vector<std::pair<const bitfield::Bits&, Value>> values;
+
+        values.reserve(_bits.size());
+        for ( const auto& f : _bits )
+            values.emplace_back(f, Value(static_cast<const char*>(v.pointer()) + f.offset, f.type, v));
+
+        return values;
+    }
+
+private:
+    const std::vector<bitfield::Bits> _bits;
+};
+
 /** Type information for type ``bool`. */
 class Bool : public detail::AtomicType<bool> {};
 
@@ -838,9 +900,9 @@ struct Field {
      * @param offset offset of the field in number bytes inside the struct
      * @param accessor function returning a pointer to a fields value
      */
-    Field(const char* name, const TypeInfo* type, std::ptrdiff_t offset, bool internal,
+    Field(const char* name, const TypeInfo* type, std::ptrdiff_t offset, bool internal, bool anonymous,
           Accessor accessor = accessor_default)
-        : name(name), type(type), offset(offset), accessor(accessor), internal(internal) {}
+        : name(name), type(type), offset(offset), accessor(accessor), internal(internal), anonymous(anonymous) {}
 
     /** Default accessor function suitable for non-optional fields. */
     static const void* accessor_default(const Value& v) { return v.pointer(); }
@@ -859,6 +921,7 @@ struct Field {
         };
     }
 
+    bool isAnonymous() const { return anonymous; }
     bool isInternal() const { return internal; }
 
     const std::string name; /**< ID of the field */
@@ -873,6 +936,7 @@ struct Field {
     const std::ptrdiff_t offset;
     const Accessor accessor;
     const bool internal;
+    const bool anonymous;
 };
 
 }; // namespace struct_
@@ -1148,6 +1212,7 @@ struct TypeInfo {
         Undefined,
         Address,
         Any,
+        Bitfield,
         Bool,
         Bytes,
         BytesIterator,
@@ -1198,6 +1263,7 @@ struct TypeInfo {
     union {
         type_info::Address* address;
         type_info::Any* any;
+        type_info::Bitfield* bitfield;
         type_info::Bool* bool_;
         type_info::Bytes* bytes;
         type_info::BytesIterator* bytes_iterator;
@@ -1256,6 +1322,10 @@ struct TypeInfo {
             tag = Any;
             any = value;
         }
+        else if constexpr ( std::is_same_v<Type, type_info::Bitfield> ) {
+            tag = Bitfield;
+            bitfield = value;
+        }
         else if constexpr ( std::is_same_v<Type, type_info::Bool> ) {
             tag = Bool;
             bool_ = value;
@@ -1450,6 +1520,10 @@ const Type* auxType(const type_info::Value& v) {
         assert(type.tag == TypeInfo::Any);
         return type.any;
     }
+    else if constexpr ( std::is_same_v<Type, type_info::Bitfield> ) {
+        assert(type.tag == TypeInfo::Bitfield);
+        return type.bitfield;
+    }
     else if constexpr ( std::is_same_v<Type, type_info::Bool> ) {
         assert(type.tag == TypeInfo::Bool);
         return type.bool_;

hilti/runtime/include/types/all.h

@@ -4,6 +4,7 @@
 
 #include <hilti/rt/types/address.h>
 #include <hilti/rt/types/any.h>
+#include <hilti/rt/types/bitfield.h>
 #include <hilti/rt/types/bool.h>
 #include <hilti/rt/types/bytes.h>
 #include <hilti/rt/types/enum.h>

hilti/runtime/include/types/bitfield.h

@@ -0,0 +1,96 @@
+// Copyright (c) 2020-2023 by the Zeek Project. See LICENSE for details.
+
+#pragma once
+
+#include <string>
+#include <tuple>
+#include <utility>
+#include <vector>
+
+#include <hilti/rt/extension-points.h>
+#include <hilti/rt/types/tuple.h>
+#include <hilti/rt/util.h>
+
+namespace hilti::rt {
+
+/// A bitfield is just a type wrapper around a tuple of the corresponding field
+/// values (including the hidden additional element storing the bitfield's
+/// original integer value). We wrap it so that we can customize the
+/// printing.
+template<typename... Ts>
+struct Bitfield {
+    std::tuple<Ts...> value;
+};
+
+template<typename... Ts>
+Bitfield<Ts...> make_bitfield(Ts&&... args) {
+    return Bitfield<Ts...>{std::make_tuple(std::forward<Ts>(args)...)};
+}
+
+namespace bitfield {
+
+template<typename Bitfield, size_t Idx>
+ptrdiff_t elementOffset() {
+    return tuple::elementOffset<decltype(Bitfield::value), Idx>();
+}
+
+} // namespace bitfield
+
+// Helper to convert tuple to a vector of the elements' string representations.
+// This version uses `to_string` for each element.
+namespace detail {
+template<class Tuple, class T = std::decay_t<std::tuple_element_t<0, std::decay_t<Tuple>>>>
+std::vector<std::string> to_vector_with_to_string(Tuple&& tuple) {
+    return std::apply(
+        [](auto&&... elems) {
+            std::vector<std::string> result;
+            result.reserve(sizeof...(elems));
+            (result.emplace_back(to_string(elems)), ...);
+            return result;
+        },
+        std::forward<Tuple>(tuple));
+}
+
+// Helper to convert tuple to a vector of the elements' string representations.
+// This version uses `to_string_for_print` for each element.
+template<class Tuple, class T = std::decay_t<std::tuple_element_t<0, std::decay_t<Tuple>>>>
+std::vector<std::string> to_vector_with_to_string_for_print(Tuple&& tuple) {
+    return std::apply(
+        [](auto&&... elems) {
+            std::vector<std::string> result;
+            result.reserve(sizeof...(elems));
+            (result.emplace_back(to_string(elems)), ...);
+            return result;
+        },
+        std::forward<Tuple>(tuple));
+}
+} // namespace detail
+
+namespace detail::adl {
+template<typename... Ts>
+inline std::string to_string(const Bitfield<Ts...>& x, adl::tag /*unused*/) {
+    // Need to remove the last, hidden element
+    auto y = to_vector_with_to_string(x.value);
+    y.pop_back();
+    return fmt("(%s)", rt::join(std::move(y), ", "));
+}
+
+template<typename... Ts>
+inline std::string to_string_for_print(const Bitfield<Ts...>& x, adl::tag /*unused*/) {
+    // Need to remove the last, hidden element
+    auto y = to_vector_with_to_string_for_print(x.value);
+    y.pop_back();
+    return fmt("(%s)", rt::join(std::move(y), ", "));
+}
+} // namespace detail::adl
+
+} // namespace hilti::rt
+
+namespace std {
+
+template<typename... Ts>
+inline std::ostream& operator<<(std::ostream& out, const hilti::rt::Bitfield<Ts...>& x) {
+    return out << hilti::rt::to_string_for_print(x);
+}
+
+} // namespace std

hilti/runtime/include/types/integer.h

@@ -343,6 +343,16 @@ inline hilti::rt::integer::safe<UINT> bits(hilti::rt::integer::safe<UINT> v, uin
     return (v & mask) >> lower;
 }
 
+/**
+ * Helper function just returning the value passed in. This is for working
+ * around an issue where our code generator produces code that, for unknown
+ * reasons, doesn't compile if the value is used directly.
+ */
+template<typename UINT>
+inline hilti::rt::integer::safe<UINT> noop(hilti::rt::integer::safe<UINT> v) {
+    return v;
+}
+
 } // namespace integer
 
 namespace detail::adl {

hilti/runtime/src/tests/type-info.cc

@@ -54,14 +54,14 @@ namespace {
 const hilti::rt::TypeInfo __ti_Test_X =
     {"Test::X", "Test::X",
      new hilti::rt::type_info::Struct(std::vector<hilti::rt::type_info::struct_::Field>(
-         {hilti::rt::type_info::struct_::Field{"i", &hilti::rt::type_info::int32, offsetof(Test::X, i), false},
-          hilti::rt::type_info::struct_::Field{"s", &hilti::rt::type_info::string, offsetof(Test::X, s), false},
-          hilti::rt::type_info::struct_::Field{"y", &type_info::__ti_Test_Y, offsetof(Test::X, y), false}}))};
+         {hilti::rt::type_info::struct_::Field{"i", &hilti::rt::type_info::int32, offsetof(Test::X, i), false, false},
+          hilti::rt::type_info::struct_::Field{"s", &hilti::rt::type_info::string, offsetof(Test::X, s), false, false},
+          hilti::rt::type_info::struct_::Field{"y", &type_info::__ti_Test_Y, offsetof(Test::X, y), false, false}}))};
 const hilti::rt::TypeInfo __ti_Test_Y =
     {"Test::Y", "Test::Y",
      new hilti::rt::type_info::Struct(std::vector<hilti::rt::type_info::struct_::Field>(
-         {hilti::rt::type_info::struct_::Field{"b", &hilti::rt::type_info::bool_, offsetof(Test::Y, b), false},
-          hilti::rt::type_info::struct_::Field{"r", &hilti::rt::type_info::real, offsetof(Test::Y, r), false}}))};
+         {hilti::rt::type_info::struct_::Field{"b", &hilti::rt::type_info::bool_, offsetof(Test::Y, b), false, false},
+          hilti::rt::type_info::struct_::Field{"r", &hilti::rt::type_info::real, offsetof(Test::Y, r), false, false}}))};
 } // namespace
 } // namespace __hlt::type_info
 
@@ -126,10 +126,10 @@ TEST_CASE("internal fields") {
 
     const TypeInfo ti = {"A", "A",
                          new type_info::Struct(
-                             {type_info::struct_::Field{"f1", &type_info::int32, offsetof(A, f1), false},
-                              type_info::struct_::Field{"f2", &type_info::string, offsetof(A, f2), false},
+                             {type_info::struct_::Field{"f1", &type_info::int32, offsetof(A, f1), false, false},
+                              type_info::struct_::Field{"f2", &type_info::string, offsetof(A, f2), false, false},
                               type_info::struct_::Field{"__internal", &type_info::bool_, offsetof(A, __internal),
-                                                        true}})};
+                                                        true, false}})};
 
     auto sx = StrongReference<A>({42, "foo", true});
     auto p = type_info::value::Parent(sx);
@@ -146,4 +146,23 @@ TEST_CASE("internal fields") {
     CHECK_EQ(s->iterate(v, true).size(), 3U);
 }
 
+TEST_CASE("anonymous fields") {
+    struct A {
+        std::string f1;
+    };
+
+    const TypeInfo ti = {"A", "A",
+                         new type_info::Struct(
+                             {type_info::struct_::Field{"f1", &type_info::int32, offsetof(A, f1), false, true}})};
+
+    auto sx = StrongReference<A>({"foo"});
+    auto p = type_info::value::Parent(sx);
+    auto v = type_info::Value(&*sx, &ti, p);
+
+    const auto s = type_info::value::auxType<type_info::Struct>(v);
+
+    CHECK_EQ(s->fields().size(), 1U);
+    CHECK(s->fields()[0].get().isAnonymous());
+}
+
 TEST_SUITE_END();

hilti/toolchain/CMakeLists.txt

@@ -50,6 +50,7 @@ set(SOURCES
     src/ast/scope.cc
     src/ast/scope-lookup.cc
     src/ast/type.cc
+    src/ast/types/bitfield.cc
     src/ast/types/enum.cc
     src/ast/types/integer.cc
     src/ast/types/library.cc

hilti/toolchain/include/ast/attribute.h

@@ -297,6 +297,23 @@ class AttributeSet : public NodeBase {
         return *s;
     }
 
+    /**
+     * Returns a new attribute set that adds one element.
+     *
+     * @param s set to add to.
+     * @param a element to add.
+     * @return `s` with `a' added
+     */
+    static AttributeSet add(optional_ref<AttributeSet> s, Attribute a) {
+        if ( s ) {
+            auto ns = *s;
+            ns.addChild(std::move(a));
+            return ns;
+        }
+        else
+            return AttributeSet({std::move(a)});
+    }
+
     /**
      * Retrieves an attribute with a given name from a set, dealing correctly
      * with an unset optional set. If multiple attributes with that tag

hilti/toolchain/include/ast/declarations/field.h

@@ -69,6 +69,7 @@ class Field : public DeclarationBase {
         return {};
     }
 
+    auto isAnonymous() const { return AttributeSet::find(attributes(), "&anonymous").has_value(); }
     auto isInternal() const { return AttributeSet::find(attributes(), "&internal").has_value(); }
     auto isOptional() const { return AttributeSet::find(attributes(), "&optional").has_value(); }
     auto isStatic() const { return AttributeSet::find(attributes(), "&static").has_value(); }

hilti/toolchain/include/ast/expressions/resolved-operator.h

@@ -68,8 +68,6 @@ class ResolvedOperatorBase : public NodeBase, public trait::isExpression, public
     bool isTemporary() const { return isLhs(); }
     /** Implements `Expression` interface. */
     const Type& type() const { return result(); }
-    /** Implements `Expression` interface. */
-    auto isEqual(const Expression& other) const { return node::isEqual(this, other); }
 
     /** Implements `Expression` interface. */
     bool isConstant() const { return type::isConstant(type()); }

hilti/toolchain/include/ast/node.h

@@ -413,8 +413,8 @@ class NodeBase : public trait::isNode {
      */
     template<typename T>
     auto children(int begin, int end) const {
-        auto end_ = (end < 0) ? _children.end() : _children.begin() + end;
-        return hilti::node::Range<T>(_children.begin() + begin, end_);
+        int end_ = (end >= 0) ? end : std::max(begin, static_cast<int>(_children.size()) + end + 1);
+        return hilti::node::Range<T>(_children.begin() + begin, _children.begin() + end_);
     }
 
     /**
@@ -426,10 +426,10 @@ class NodeBase : public trait::isNode {
      * @return vector containing child references from `start` to `end`
      */
     auto childRefs(int begin, int end) {
-        auto end_ = (end < 0) ? _children.end() : _children.begin() + end;
+        int end_ = (end >= 0) ? end : std::max(begin, static_cast<int>(_children.size()) + end + 1);
 
         std::vector<NodeRef> refs;
-        for ( auto c = _children.begin(); c != end_; c = std::next(c) )
+        for ( auto c = _children.begin(); c != _children.begin() + end_; c = std::next(c) )
             refs.emplace_back(*c);
 
         return refs;