Clean and reorganize OffsetString and StringPool

[Hind-M]

• Move definitions to cpp files
• Remove unnecessary headers inclusions
• Remove OffsetString::offset_t and StringPool::offset_t in favor of entity::position_t (same thing, different names). This will avoid including offset_string.hpp and string_pool.hpp just for these while types.hpp is already there
• Define types in a specific file to be included where needed instead of the whole types.hpp (to be done in an upcoming PR)

[alexowens90]

The hpp->cpp changes all look good. What is the motivation for changing StringPool::offset_t and OffsetString::offset_t to entity::position_t everywhere?

[Hind-M]

The hpp->cpp changes all look good. What is the motivation for changing StringPool::offset_t and OffsetString::offset_t to entity::position_t everywhere?

To simplify, and because using StringPool::offset_t and OffsetString::offset_t involves including the corresponding headers offset_string.hpp and string_pool.hpp while types.hpp is already included (which contains entity::position_t which is the same thing as X::offset_t).
A good next step would be to move the position_t-like types from types.hpp to a separate file to be included instead of shipping the whole types.hpp.

[alexowens90]

The hpp->cpp changes all look good. What is the motivation for changing StringPool::offset_t and OffsetString::offset_t to entity::position_t everywhere?

To simplify, and because using StringPool::offset_t and OffsetString::offset_t involves including the corresponding headers offset_string.hpp and string_pool.hpp while types.hpp is already included (which contains entity::position_t which is the same thing as X::offset_t). A good next step would be to move the position_t-like types from types.hpp to a separate file to be included instead of shipping the whole types.hpp.

There might be a reason the separate types were originally created, maybe @willdealtry can weigh in?

[willdealtry]

The hpp->cpp changes all look good. What is the motivation for changing StringPool::offset_t and OffsetString::offset_t to entity::position_t everywhere?

To simplify, and because using StringPool::offset_t and OffsetString::offset_t involves including the corresponding headers offset_string.hpp and string_pool.hpp while types.hpp is already included (which contains entity::position_t which is the same thing as X::offset_t). A good next step would be to move the position_t-like types from types.hpp to a separate file to be included instead of shipping the whole types.hpp.

There might be a reason the separate types were originally created, maybe @willdealtry can weigh in?

I don't think it hugely matters. I wrote the string pool in the first or second week, it's possible that position_t didn't exist at that point. The purpose is similar enough (it's an offset in the buffer) that I think we can just use one.