Skip to content

Commit

Permalink
Fix ReactCommon Break for Windows (#33047)
Browse files Browse the repository at this point in the history
Summary:
Changes to MapBuffer code in aaff15c...d287598 broke build for Windows. Errors included incompatible type conversions, the use of `__attribute__(__packed__)` which is only supported by GCC and Clang, and the usage of designated initializers which are only supported on C++20.

Changes here restore build on Windows.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[General] [Fixed] - Fix build break on Windows with ReactCommon

Pull Request resolved: #33047

Test Plan: React Native project built on Windows and passes react-native-windows repository pipeline. These edits are currently merged into the main branch of react-native-windows.

Reviewed By: ShikaSD

Differential Revision: D34101367

Pulled By: philIip

fbshipit-source-id: 1596365c2e92f377c6375805b33de5e1c7b78e66
  • Loading branch information
chiaramooney authored and facebook-github-bot committed Feb 9, 2022
1 parent 216ac27 commit 42b3917
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 14 deletions.
10 changes: 5 additions & 5 deletions ReactCommon/react/renderer/mapbuffer/MapBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ MapBuffer::MapBuffer(std::vector<uint8_t> data) : bytes_(std::move(data)) {
}
}

uint32_t MapBuffer::getKeyBucket(Key key) const {
uint32_t lo = 0;
uint32_t hi = count_ - 1;
int32_t MapBuffer::getKeyBucket(Key key) const {
int32_t lo = 0;
int32_t hi = count_ - 1;
while (lo <= hi) {
uint32_t mid = (lo + hi) >> 1;
int32_t mid = (lo + hi) >> 1;

Key midVal =
*reinterpret_cast<Key const *>(bytes_.data() + bucketOffset(mid));
Expand Down Expand Up @@ -112,7 +112,7 @@ MapBuffer MapBuffer::getMapBuffer(Key key) const {
return MapBuffer(std::move(value));
}

uint32_t MapBuffer::size() const {
size_t MapBuffer::size() const {
return bytes_.size();
}

Expand Down
8 changes: 5 additions & 3 deletions ReactCommon/react/renderer/mapbuffer/MapBuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,16 @@ class MapBuffer {
uint32_t bufferSize; // Amount of bytes used to store the map in memory
};

struct __attribute__((__packed__)) Bucket {
#pragma pack(push, 1)
struct Bucket {
Key key;
uint16_t type;
uint64_t data;

Bucket(Key key, uint16_t type, uint64_t data)
: key(key), type(type), data(data) {}
};
#pragma pack(pop)

static_assert(sizeof(Header) == 8, "MapBuffer header size is incorrect.");
static_assert(sizeof(Bucket) == 12, "MapBuffer bucket size is incorrect.");
Expand Down Expand Up @@ -124,7 +126,7 @@ class MapBuffer {
// TODO T83483191: review this declaration
MapBuffer getMapBuffer(MapBuffer::Key key) const;

uint32_t size() const;
size_t size() const;

uint8_t const *data() const;

Expand All @@ -140,7 +142,7 @@ class MapBuffer {
// returns the relative offset of the first byte of dynamic data
int32_t getDynamicDataOffset() const;

uint32_t getKeyBucket(MapBuffer::Key key) const;
int32_t getKeyBucket(MapBuffer::Key key) const;

friend ReadableMapBuffer;
};
Expand Down
12 changes: 7 additions & 5 deletions ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace facebook {
namespace react {

constexpr uint32_t INT_SIZE = sizeof(uint32_t);
constexpr double DOUBLE_SIZE = sizeof(double);
constexpr uint32_t DOUBLE_SIZE = sizeof(double);
constexpr uint32_t MAX_BUCKET_VALUE_SIZE = sizeof(uint64_t);

MapBuffer MapBufferBuilder::EMPTY() {
Expand All @@ -23,6 +23,8 @@ MapBuffer MapBufferBuilder::EMPTY() {

MapBufferBuilder::MapBufferBuilder(uint32_t initialSize) {
buckets_.reserve(initialSize);
header_.count = 0;
header_.bufferSize = 0;
}

void MapBufferBuilder::storeKeyValue(
Expand Down Expand Up @@ -76,7 +78,7 @@ void MapBufferBuilder::putInt(MapBuffer::Key key, int32_t value) {
}

void MapBufferBuilder::putString(MapBuffer::Key key, std::string const &value) {
int32_t strSize = value.size();
auto strSize = value.size();
const char *strData = value.data();

// format [length of string (int)] + [Array of Characters in the string]
Expand All @@ -94,7 +96,7 @@ void MapBufferBuilder::putString(MapBuffer::Key key, std::string const &value) {
}

void MapBufferBuilder::putMapBuffer(MapBuffer::Key key, MapBuffer const &map) {
int32_t mapBufferSize = map.size();
auto mapBufferSize = map.size();

auto offset = dynamicData_.size();

Expand Down Expand Up @@ -122,9 +124,9 @@ MapBuffer MapBufferBuilder::build() {
// Create buffer: [header] + [key, values] + [dynamic data]
auto bucketSize = buckets_.size() * sizeof(MapBuffer::Bucket);
auto headerSize = sizeof(MapBuffer::Header);
uint32_t bufferSize = headerSize + bucketSize + dynamicData_.size();
auto bufferSize = headerSize + bucketSize + dynamicData_.size();

header_.bufferSize = bufferSize;
header_.bufferSize = static_cast<uint32_t>(bufferSize);

if (needsSort_) {
std::sort(buckets_.begin(), buckets_.end(), compareBuckets);
Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/react/renderer/mapbuffer/MapBufferBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class MapBufferBuilder {
MapBuffer build();

private:
MapBuffer::Header header_ = {.count = 0, .bufferSize = 0};
MapBuffer::Header header_;

std::vector<MapBuffer::Bucket> buckets_{};

Expand Down

0 comments on commit 42b3917

Please sign in to comment.