Skip to content

Commit

Permalink
fix: maxos build breaks due to imprecise use of constexpr
Browse files Browse the repository at this point in the history
* also add comments and clarification to the alignment requirements in fb_parser's parser::parse()
  • Loading branch information
lokitoth committed Jan 31, 2024
1 parent 85f8999 commit 7ea997a
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
2 changes: 1 addition & 1 deletion vowpalwabbit/core/include/vw/core/io_buf.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ struct desired_align
align_t offset;

template <typename T>
static constexpr desired_align align_for(align_t offset = 0)
static desired_align align_for(align_t offset = 0)
{
return desired_align{compute_align<T>(), offset};
}
Expand Down
10 changes: 7 additions & 3 deletions vowpalwabbit/fb_parser/src/parse_example_flatbuffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ int parser::parse(io_buf& buf, uint8_t* buffer_pointer, VW::experimental::api_st
<< " ^^ -4 is the size of the flatbuffer prefix, which we read explicitly."; \
}

using size_prefix_t = uint32_t;
constexpr std::size_t EXPECTED_ALIGNMENT = 8; // this is where FB expects the size-prefixed FB to be aligned
constexpr std::size_t EXPECTED_OFFSET = sizeof(uint32_t); //
constexpr std::size_t EXPECTED_OFFSET = sizeof(size_prefix_t); // when we manually read the size-prefix, the data
// block of the flat buffer is offset by its size

desired_align align_prefixed = {EXPECTED_ALIGNMENT, 0};
desired_align align_data = {EXPECTED_ALIGNMENT, EXPECTED_OFFSET};
Expand All @@ -76,14 +78,16 @@ int parser::parse(io_buf& buf, uint8_t* buffer_pointer, VW::experimental::api_st
}

char* line = nullptr;
auto len = buf.buf_read(line, sizeof(uint32_t), align_prefixed);
auto len = buf.buf_read(line, sizeof(size_prefix_t), align_prefixed); // the prefixed flatbuffer block should be
// aligned to 8 bytse, no offset

if (len < sizeof(uint32_t)) { RETURN_ERROR(status, nothing_to_parse); }

_object_size = flatbuffers::ReadScalar<flatbuffers::uoffset_t>(line);

// read one object, object size defined by the read prefix
buf.buf_read(line, _object_size, align_data);
buf.buf_read(line, _object_size, align_data); // the data block of the flatbuffer should be aligned to 8 bytes,
// offset by the size of the prefix

RETURN_IF_ALIGN_ERROR(align_data, line, _num_example_roots);

Expand Down

0 comments on commit 7ea997a

Please sign in to comment.