Example of `$size_in_bits` from documentation fails to compile

[josh-conner]

The example provided in https://github.com/google/emboss/blob/master/doc/language-reference.md#size_in_bits-size-in-bits (using bits FixedSize and struct Envelope gives the following error when built with emboss:

error: Fixed-size type 'FixedSize' cannot be placed in field of size 64 bits; requires 4 bits.
  0  [+8]  FixedSize  padded_payload
           ^^^^^^^^^

This seems to imply that FixedSize needs to be padded out to a whole byte increment. We should either:

• relax the restrictions around padding (auto-pad by default?), or
• fix the example to include the necessary padding

[BenjaminLawson]

I think this is a bug and no padding in bits fields should be necessary.

[BenjaminLawson]

I found this in constraints.py:

 if field_is_atomic and element_size is not None:
    # If the field has a fixed size, and the (atomic) type contained therein is
    # also fixed size, then the sizes should match.
    #
    # TODO(bolms): Maybe change the case where the field is bigger than
    # necessary into a warning?
    if (field_max_size == field_min_size and
        (element_size > field_max_size or
         (element_size < field_min_size and not type_is_anonymous))):
      errors.append([
          error.error(
              source_file_name, type_ir.source_location,
              "Fixed-size {} cannot be placed in field of size {} bits; "
              "requires {} bits.".format(
                  _render_type(type_ir, ir), field_max_size, element_size))
      ])
      return

It looks like Ben has already considered making this error a warning.

[BenjaminLawson]

IMO it shouldn't even be a warning. Many of our bits types are smaller than the fields they are placed in.

[reventlov]

As currently designed, this is a bug in the documentation. I would have to scour through the internal history, but I'm guessing that I extended the check to bits (and not just external types like UInt) after I wrote that.

I don't think there is a way to have a bits in a mis-sized field, so the whole explanation of intrinsic vs extrinsic sizing is moot right now.

That said:

The main point of that check is that if you have a structure that is (always) 32 bits in size, and you stick it in a field that is statically sized at 64 bits, that seems likely to be a bug in your .emb, and you can always silence the error by shrinking the field or wrapping the field in an anonymous bits.

It's definitely a bit annoying if you have, say, a 28-bit bits structure that you want to just drop into a struct:

bits Foo:
  0  [+8]  UInt  a
  24 [+4]  UInt  b
  # 28 [+4]  _  reserved bits

struct Bar:
  # 0 [+4]  Foo  foo  # Won't compile
  0 [+4]  bits:  # Anonymous `bits` are allowed to be smaller than their field size
    0 [+28]  Foo  foo

... so maybe the compiler shouldn't emit anything if you put bits in an oversized field.[^1]

I think it should still emit at least a warning[^2] if you stick something like a UInt into an oversized field:

struct Foo:
  0 [+8]  UInt:32  a  # Did you intend this to be a 4-byte or 8-byte UInt?

[^1]: Note to any implementor: there is a possibly-confusing situation when a bits field is big endian and the field is oversized by >=1 byte, in that the bits should be placed at the end of the allocated space. One could handle this by (doing the equivalent of) silently wrapping the field in an anonymous bits:

```
struct Foo:
  0 [+4]  TwentyBit  bit

=>

struct Foo:
  0 [+4]  bits:
    0 [+20]  TwentyBit bit  # actually sits in bytes 1-3
```

[^2]: The compiler has no actual facility for emitting warnings right now, and adding that facility is a chunk of refactoring, since everything pretty much just passes around lists of errors and checks "success" by checking whether the error list is empty. That's a bit of legacy from when I started and thought "oh, this will be a three-week project" and not "I guess I'm building a significant chunk of a Real Compiler."