Restore ability to upgrade from List(Bool) to List(T) where T is a struct whose…

Restore ability to upgrade from List(Bool) to List(T) where T is a struct whose @0 field is of type Bool.  I previously disallowed this to reduce complexity, but it turned out to actually increase complexity.

c++/src/capnproto/encoding-test.c++

@@ -435,7 +435,8 @@ TEST(Encoding, SmallStructLists) {
   EXPECT_EQ(0u, sl.getStructListList().size());
 
   { auto l = sl.initList0 (2); l[0].setF(Void::VOID);        l[1].setF(Void::VOID); }
-  { auto l = sl.initList1 (2); l[0].setF(true);              l[1].setF(false); }
+  { auto l = sl.initList1 (4); l[0].setF(true);              l[1].setF(false);
+                               l[2].setF(true);              l[3].setF(true); }
   { auto l = sl.initList8 (2); l[0].setF(123u);              l[1].setF(45u); }
   { auto l = sl.initList16(2); l[0].setF(12345u);            l[1].setF(6789u); }
   { auto l = sl.initList32(2); l[0].setF(123456789u);        l[1].setF(234567890u); }
@@ -482,6 +483,139 @@ TEST(Encoding, SmallStructLists) {
   }
 }
 
+// =======================================================================================
+
+TEST(Encoding, ListUpgrade) {
+  MallocMessageBuilder builder;
+  auto root = builder.initRoot<test::TestObject>();
+
+  root.initObjectField<List<uint16_t>>(3).copyFrom({12, 34, 56});
+
+  checkList(root.getObjectField<List<uint8_t>>(), {12, 34, 56});
+
+  {
+    auto l = root.getObjectField<List<test::TestLists::Struct8>>();
+    ASSERT_EQ(3u, l.size());
+    EXPECT_EQ(12u, l[0].getF());
+    EXPECT_EQ(34u, l[1].getF());
+    EXPECT_EQ(56u, l[2].getF());
+  }
+
+  checkList(root.getObjectField<List<uint16_t>>(), {12, 34, 56});
+
+  auto reader = root.asReader();
+
+  checkList(reader.getObjectField<List<uint8_t>>(), {12, 34, 56});
+
+  {
+    auto l = reader.getObjectField<List<test::TestLists::Struct8>>();
+    ASSERT_EQ(3u, l.size());
+    EXPECT_EQ(12u, l[0].getF());
+    EXPECT_EQ(34u, l[1].getF());
+    EXPECT_EQ(56u, l[2].getF());
+  }
+
+  try {
+    reader.getObjectField<List<uint32_t>>();
+    ADD_FAILURE() << "Expected exception.";
+  } catch (const Exception& e) {
+    // expected
+  }
+
+  {
+    auto l = reader.getObjectField<List<test::TestLists::Struct32>>();
+    ASSERT_EQ(3u, l.size());
+
+    // These should return default values because the structs aren't big enough.
+    EXPECT_EQ(0u, l[0].getF());
+    EXPECT_EQ(0u, l[1].getF());
+    EXPECT_EQ(0u, l[2].getF());
+  }
+
+  checkList(reader.getObjectField<List<uint16_t>>(), {12, 34, 56});
+}
+
+TEST(Encoding, BitListDowngrade) {
+  MallocMessageBuilder builder;
+  auto root = builder.initRoot<test::TestObject>();
+
+  root.initObjectField<List<uint16_t>>(4).copyFrom({0x1201u, 0x3400u, 0x5601u, 0x7801u});
+
+  checkList(root.getObjectField<List<bool>>(), {true, false, true, true});
+
+  {
+    auto l = root.getObjectField<List<test::TestLists::Struct1>>();
+    ASSERT_EQ(4u, l.size());
+    EXPECT_TRUE(l[0].getF());
+    EXPECT_FALSE(l[1].getF());
+    EXPECT_TRUE(l[2].getF());
+    EXPECT_TRUE(l[3].getF());
+  }
+
+  checkList(root.getObjectField<List<uint16_t>>(), {0x1201u, 0x3400u, 0x5601u, 0x7801u});
+
+  auto reader = root.asReader();
+
+  checkList(reader.getObjectField<List<bool>>(), {true, false, true, true});
+
+  {
+    auto l = reader.getObjectField<List<test::TestLists::Struct1>>();
+    ASSERT_EQ(4u, l.size());
+    EXPECT_TRUE(l[0].getF());
+    EXPECT_FALSE(l[1].getF());
+    EXPECT_TRUE(l[2].getF());
+    EXPECT_TRUE(l[3].getF());
+  }
+
+  checkList(reader.getObjectField<List<uint16_t>>(), {0x1201u, 0x3400u, 0x5601u, 0x7801u});
+}
+
+TEST(Encoding, BitListUpgrade) {
+  MallocMessageBuilder builder;
+  auto root = builder.initRoot<test::TestObject>();
+
+  root.initObjectField<List<bool>>(4).copyFrom({true, false, true, true});
+
+  {
+    auto l = root.getObjectField<List<test::TestFieldZeroIsBit>>();
+    ASSERT_EQ(4u, l.size());
+    EXPECT_TRUE(l[0].getBit());
+    EXPECT_FALSE(l[1].getBit());
+    EXPECT_TRUE(l[2].getBit());
+    EXPECT_TRUE(l[3].getBit());
+  }
+
+  auto reader = root.asReader();
+
+  try {
+    reader.getObjectField<List<uint8_t>>();
+    ADD_FAILURE() << "Expected exception.";
+  } catch (const Exception& e) {
+    // expected
+  }
+
+  {
+    auto l = reader.getObjectField<List<test::TestFieldZeroIsBit>>();
+    ASSERT_EQ(4u, l.size());
+    EXPECT_TRUE(l[0].getBit());
+    EXPECT_FALSE(l[1].getBit());
+    EXPECT_TRUE(l[2].getBit());
+    EXPECT_TRUE(l[3].getBit());
+
+    // Other fields are defaulted.
+    EXPECT_TRUE(l[0].getSecondBit());
+    EXPECT_TRUE(l[1].getSecondBit());
+    EXPECT_TRUE(l[2].getSecondBit());
+    EXPECT_TRUE(l[3].getSecondBit());
+    EXPECT_EQ(123u, l[0].getThirdField());
+    EXPECT_EQ(123u, l[1].getThirdField());
+    EXPECT_EQ(123u, l[2].getThirdField());
+    EXPECT_EQ(123u, l[3].getThirdField());
+  }
+
+  checkList(reader.getObjectField<List<bool>>(), {true, false, true, true});
+}
+
 // =======================================================================================
 // Tests of generated code, not really of the encoding.
 // TODO(cleanup):  Move to a different test?

c++/src/capnproto/message.c++

@@ -53,7 +53,7 @@ internal::StructReader MessageReader::getRootInternal() {
   VALIDATE_INPUT(segment != nullptr &&
       segment->containsInterval(segment->getStartPtr(), segment->getStartPtr() + 1),
       "Message did not contain a root pointer.") {
-    return internal::StructReader::readEmpty();
+    return internal::StructReader();
   }
 
   return internal::StructReader::readRoot(segment->getStartPtr(), segment, options.nestingLimit);

c++/src/capnproto/test-util.c++

@@ -329,7 +329,7 @@ void genericInitListDefaults(Builder builder) {
   auto lists = builder.initLists();
 
   lists.initList0(2);
-  lists.initList1(2);
+  lists.initList1(4);
   lists.initList8(2);
   lists.initList16(2);
   lists.initList32(2);
@@ -340,6 +340,8 @@ void genericInitListDefaults(Builder builder) {
   lists.getList0()[1].setF(Void::VOID);
   lists.getList1()[0].setF(true);
   lists.getList1()[1].setF(false);
+  lists.getList1()[2].setF(true);
+  lists.getList1()[3].setF(true);
   lists.getList8()[0].setF(123u);
   lists.getList8()[1].setF(45u);
   lists.getList16()[0].setF(12345u);
@@ -380,7 +382,7 @@ void genericCheckListDefaults(Reader reader) {
   auto lists = reader.getLists();
 
   ASSERT_EQ(2u, lists.getList0().size());
-  ASSERT_EQ(2u, lists.getList1().size());
+  ASSERT_EQ(4u, lists.getList1().size());
   ASSERT_EQ(2u, lists.getList8().size());
   ASSERT_EQ(2u, lists.getList16().size());
   ASSERT_EQ(2u, lists.getList32().size());
@@ -391,6 +393,8 @@ void genericCheckListDefaults(Reader reader) {
   EXPECT_EQ(Void::VOID, lists.getList0()[1].getF());
   EXPECT_TRUE(lists.getList1()[0].getF());
   EXPECT_FALSE(lists.getList1()[1].getF());
+  EXPECT_TRUE(lists.getList1()[2].getF());
+  EXPECT_TRUE(lists.getList1()[3].getF());
   EXPECT_EQ(123u, lists.getList8()[0].getF());
   EXPECT_EQ(45u, lists.getList8()[1].getF());
   EXPECT_EQ(12345u, lists.getList16()[0].getF());

c++/src/capnproto/test.capnp

@@ -308,10 +308,16 @@ struct TestLists {
   structListList @9 :List(List(TestAllTypes));
 }
 
+struct TestFieldZeroIsBit {
+  bit @0 :Bool;
+  secondBit @1 :Bool = true;
+  thirdField @2 :UInt8 = 123;
+}
+
 struct TestListDefaults {
   lists @0 :TestLists = (
       list0  = [(f = void), (f = void)],
-      list1  = [(f = true), (f = false)],
+      list1  = [(f = true), (f = false), (f = true), (f = true)],
       list8  = [(f = 123), (f = 45)],
       list16 = [(f = 12345), (f = 6789)],
       list32 = [(f = 123456789), (f = 234567890)],

c++/src/capnproto/type-safety.h

@@ -462,6 +462,19 @@ struct Id {
 // =======================================================================================
 // Units
 
+template <typename T> constexpr bool isIntegral() { return false; }
+template <> constexpr bool isIntegral<char>() { return true; }
+template <> constexpr bool isIntegral<signed char>() { return true; }
+template <> constexpr bool isIntegral<short>() { return true; }
+template <> constexpr bool isIntegral<int>() { return true; }
+template <> constexpr bool isIntegral<long>() { return true; }
+template <> constexpr bool isIntegral<long long>() { return true; }
+template <> constexpr bool isIntegral<unsigned char>() { return true; }
+template <> constexpr bool isIntegral<unsigned short>() { return true; }
+template <> constexpr bool isIntegral<unsigned int>() { return true; }
+template <> constexpr bool isIntegral<unsigned long>() { return true; }
+template <> constexpr bool isIntegral<unsigned long long>() { return true; }
+
 template <typename Number, typename Unit1, typename Unit2>
 class UnitRatio {
   // A multiplier used to convert Quantities of one unit to Quantities of another unit.  See
@@ -470,6 +483,8 @@ class UnitRatio {
   // Construct this type by dividing one Quantity by another of a different unit.  Use this type
   // by multiplying it by a Quantity, or dividing a Quantity by it.
 
+  static_assert(isIntegral<Number>(), "Underlying type for UnitRatio must be integer.");
+
 public:
   inline UnitRatio() {}
 
@@ -529,12 +544,14 @@ private:
   friend class UnitRatio;
 
   template <typename N1, typename N2, typename U1, typename U2>
-  friend inline constexpr decltype(N1(1) * N2(1)) operator*(N1, UnitRatio<N2, U1, U2>);
+  friend inline constexpr UnitRatio<decltype(N1(1) * N2(1)), U1, U2>
+      operator*(N1, UnitRatio<N2, U1, U2>);
 };
 
 template <typename N1, typename N2, typename U1, typename U2>
-inline constexpr decltype(N1(1) * N2(1)) operator*(N1 n, UnitRatio<N2, U1, U2> r) {
-  return n * r.unit1PerUnit2;
+inline constexpr UnitRatio<decltype(N1(1) * N2(1)), U1, U2>
+    operator*(N1 n, UnitRatio<N2, U1, U2> r) {
+  return UnitRatio<decltype(N1(1) * N2(1)), U1, U2>(n * r.unit1PerUnit2);
 }
 
 template <typename Number, typename Unit>
@@ -581,6 +598,8 @@ class Quantity {
   //     waitFor(3 * MINUTES);
   //   }
 
+  static_assert(isIntegral<Number>(), "Underlying type for Quantity must be integer.");
+
 public:
   inline constexpr Quantity() {}
 
@@ -605,11 +624,13 @@ public:
   template <typename OtherNumber>
   inline constexpr Quantity<decltype(Number(1) * OtherNumber(1)), Unit>
       operator*(OtherNumber other) const {
+    static_assert(isIntegral<OtherNumber>(), "Multiplied Quantity by non-integer.");
     return Quantity<decltype(Number(1) * other), Unit>(value * other);
   }
   template <typename OtherNumber>
   inline constexpr Quantity<decltype(Number(1) / OtherNumber(1)), Unit>
       operator/(OtherNumber other) const {
+    static_assert(isIntegral<OtherNumber>(), "Divided Quantity by non-integer.");
     return Quantity<decltype(Number(1) / other), Unit>(value / other);
   }
   template <typename OtherNumber>

compiler/src/Compiler.hs

@@ -708,8 +708,7 @@ packUnion :: UnionDesc -> PackingState -> Map.Map Integer UnionPackingState
 packUnion _ state unionState = (DataOffset Size16 offset, newState, unionState) where
     (offset, newState) = packData Size16 state
 
-stripHolesFromFirstWord Size1 _ = error "can't get this far"
-stripHolesFromFirstWord Size8 _ = Size8  -- Don't reduce to less than a byte.
+stripHolesFromFirstWord Size1 _ = Size1  -- Stop at a bit.
 stripHolesFromFirstWord size holes = let
     nextSize = pred size
     in case Map.lookup nextSize holes of
@@ -744,6 +743,7 @@ enforceFixed Nothing sizes = return sizes
 enforceFixed (Just (Located pos (requestedDataSize, requestedPointerCount)))
         (actualDataSize, actualPointerCount) = do
     validatedRequestedDataSize <- case requestedDataSize of
+        1 -> return DataSection1
         8 -> return DataSection8
         16 -> return DataSection16
         32 -> return DataSection32

compiler/src/CxxGenerator.hs

@@ -382,7 +382,7 @@ structContext parent desc = mkStrContext context where
     context "structReferenceCount" = MuVariable $ structPointerCount desc
     context "structPreferredListEncoding" = case (structDataSize desc, structPointerCount desc) of
         (DataSectionWords 0, 0) -> MuVariable "VOID"
-        (DataSection1, 0) -> MuVariable "BYTE"
+        (DataSection1, 0) -> MuVariable "BIT"
         (DataSection8, 0) -> MuVariable "BYTE"
         (DataSection16, 0) -> MuVariable "TWO_BYTES"
         (DataSection32, 0) -> MuVariable "FOUR_BYTES"

compiler/src/Semantics.hs

@@ -318,9 +318,9 @@ fieldSize (InlineStructType StructDesc { structDataSize = ds, structPointerCount
 fieldSize (InterfaceType _) = SizeReference
 fieldSize (ListType _) = SizeReference
 fieldSize (InlineListType element size) = let
-    -- We intentionally do not allow single-bit lists because most CPUs cannot address bits.
     minDataSectionForBits bits
         | bits <= 0 = DataSectionWords 0
+        | bits <= 1 = DataSection1
         | bits <= 8 = DataSection8
         | bits <= 16 = DataSection16
         | bits <= 32 = DataSection32

doc/encoding.md

@@ -114,11 +114,12 @@ A list value is encoded as a pointer to a flat array of values.
 The pointed-to values are tightly-packed.  In particular, `Bool`s are packed bit-by-bit in
 little-endian order (the first bit is the least-significant bit of the first byte).
 
-Lists of structs use the smallest element size in which the struct can fit, except that single-bit
-structs are not allowed.  So, a list of structs that each contain two `UInt8` fields and nothing
-else could be encoded with C = 3 (2-byte elements).  A list of structs that each contain a single
-`Text` field would be encoded as C = 6 (pointer elements).  A list structs which are each more than
-one word in size must be be encoded using C = 7 (composite).
+Lists of structs use the smallest element size in which the struct can fit.  So, a
+list of structs that each contain two `UInt8` fields and nothing else could be encoded with C = 3
+(2-byte elements).  A list of structs that each contain a single `Text` field would be encoded as
+C = 6 (pointer elements).  A list of structs that each contain a single `Bool` field would be
+encoded using C = 1 (1-bit elements).  A list structs which are each more than one word in size
+must be be encoded using C = 7 (composite).
 
 When C = 7, the elements of the list are fixed-width composite values -- usually, structs.  In
 this case, the list content is prefixed by a "tag" word that describes each individual element.
@@ -136,7 +137,7 @@ elements being fixed-size lists rather than structs.  In this case, the tag woul
 pointer rather than a struct pointer.  As of this writing, no such feature has been implemented.
 
 Notice that because a small struct is encoded as if it were a primitive value, this means that
-if you have a field of type `List(T)` where `T` is a primitive or blob type (other than `Bool`), it
+if you have a field of type `List(T)` where `T` is a primitive or blob type, it
 is possible to change that field to `List(U)` where `U` is a struct whose `@0` field has type `T`,
 without breaking backwards-compatibility.  This comes in handy when you discover too late that you
 need to associate some extra data with each value in a primitive list -- instead of using parallel

doc/language.md

@@ -489,12 +489,11 @@ A protocol can be changed in the following ways without breaking backwards-compa
   parameter list and must have default values.
 * Any symbolic name can be changed, as long as the ordinal numbers stay the same.
 * Types definitions can be moved to different scopes.
-* A field of type `List(T)`, where `T` is a primitive type (except `Bool`), non-inline blob, or
+* A field of type `List(T)`, where `T` is a primitive type, non-inline blob, or
   non-inline list, may be changed to type `List(U)`, where `U` is a struct type whose `@0` field is
   of type `T`.  This rule is useful when you realize too late that you need to attach some extra
   data to each element of your list.  Without this rule, you would be stuck defining parallel
-  lists, which are ugly and error-prone.  (`List(Bool)` does not support this transformation
-  because it would be difficult to implement given that booleans are packed 8 to the byte.)
+  lists, which are ugly and error-prone.
 * A struct that is not already `fixed` can be made `fixed`.  However, once a struct is declared
   `fixed`, the declaration cannot be removed or changed, as this would change the layout of `Inline`
   uses of the struct.