Skip to content

Commit

Permalink
Clean-up SlicedData in C++ (#2575)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bernardnormier
bernardnormier authored Jul 26, 2024
1 parent 39e2ce4 commit dc2e185
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 73 deletions.
54 changes: 42 additions & 12 deletions cpp/include/Ice/SlicedData.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@

#include <string>

#if defined(__clang__)
# pragma clang diagnostic push
# pragma clang diagnostic ignored "-Wshadow-field-in-constructor"
#elif defined(__GNUC__)
# pragma GCC diagnostic push
# pragma GCC diagnostic ignored "-Wshadow"
#endif

namespace Ice
{
/**
Expand All @@ -22,17 +30,17 @@ namespace Ice
/**
* The Slice type ID for this slice.
*/
std::string typeId;
const std::string typeId;

/**
* The Slice compact type ID for this slice.
*/
int compactId;
const int compactId;

/**
* The encoded bytes for this slice, including the leading size integer.
*/
std::vector<std::byte> bytes;
const std::vector<std::byte> bytes;

/**
* The class instances referenced by this slice.
Expand All @@ -42,22 +50,44 @@ namespace Ice
/**
* Whether or not the slice contains optional members.
*/
bool hasOptionalMembers;
const bool hasOptionalMembers;

/**
* Whether or not this is the last slice.
*/
bool isLastSlice;
const bool isLastSlice;

/**
* Constructs a new SliceInfo instance.
* @param typeId The Slice type ID for this slice.
* @param compactId The Slice compact type ID for this slice.
* @param bytes The encoded bytes for this slice.
* @param hasOptionalMembers Whether or not the slice contains optional members.
* @param isLastSlice Whether or not this is the last slice.
*/
SliceInfo(
std::string typeId,
int compactId,
std::vector<std::byte> bytes,
bool hasOptionalMembers,
bool isLastSlice) noexcept
: typeId(std::move(typeId)),
compactId(compactId),
bytes(std::move(bytes)),
hasOptionalMembers(hasOptionalMembers),
isLastSlice(isLastSlice)
{
}
};

/**
* Holds the slices of unknown types.
* \headerfile Ice/Ice.h
*/
class ICE_API SlicedData
class ICE_API SlicedData final
{
public:
SlicedData(const SliceInfoSeq&);
SlicedData(SliceInfoSeq slices) noexcept;

/** The slices of unknown types. */
const SliceInfoSeq slices;
Expand All @@ -72,20 +102,20 @@ namespace Ice
* Represents an instance of an unknown type.
* \headerfile Ice/Ice.h
*/
class ICE_API UnknownSlicedValue : public Value
class ICE_API UnknownSlicedValue final : public Value
{
public:
/**
* Constructs the placeholder instance.
* @param unknownTypeId The Slice type ID of the unknown value.
*/
UnknownSlicedValue(const std::string& unknownTypeId);
UnknownSlicedValue(std::string unknownTypeId) noexcept;

/**
* Determine the Slice type ID associated with this instance.
* @return The type ID supplied to the constructor.
*/
const char* ice_id() const noexcept override;
const char* ice_id() const noexcept final;

/**
* Clones this object.
Expand All @@ -98,11 +128,11 @@ namespace Ice

protected:
/// \cond INTERNAL
ValuePtr _iceCloneImpl() const override;
ValuePtr _iceCloneImpl() const final;
/// \endcond

private:
const std::string _unknownTypeId;
std::string _unknownTypeId;
};
}

Expand Down
21 changes: 13 additions & 8 deletions cpp/src/Ice/InputStream.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2306,23 +2306,28 @@ Ice::InputStream::EncapsDecoder11::skipSlice()
//
if (_current->sliceType == ValueSlice)
{
SliceInfoPtr info = make_shared<SliceInfo>();
info->typeId = _current->typeId;
info->compactId = _current->compactId;
info->hasOptionalMembers = _current->sliceFlags & FLAG_HAS_OPTIONAL_MEMBERS;
info->isLastSlice = _current->sliceFlags & FLAG_IS_LAST_SLICE;
if (info->hasOptionalMembers)
bool hasOptionalMembers = _current->sliceFlags & FLAG_HAS_OPTIONAL_MEMBERS;
vector<byte> bytes;
if (hasOptionalMembers)
{
//
// Don't include the optional member end marker. It will be re-written by
// endSlice when the sliced data is re-written.
//
vector<byte>(start, _stream->i - 1).swap(info->bytes);
bytes = vector<byte>(start, _stream->i - 1);
}
else
{
vector<byte>(start, _stream->i).swap(info->bytes);
bytes = vector<byte>(start, _stream->i);
}

SliceInfoPtr info = make_shared<SliceInfo>(
_current->typeId,
_current->compactId,
std::move(bytes),
hasOptionalMembers,
_current->sliceFlags & FLAG_IS_LAST_SLICE);

_current->slices.push_back(info);
}

Expand Down
4 changes: 2 additions & 2 deletions cpp/src/Ice/SlicedData.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
using namespace std;
using namespace Ice;

Ice::SlicedData::SlicedData(const SliceInfoSeq& seq) : slices(seq) {}
Ice::SlicedData::SlicedData(SliceInfoSeq seq) noexcept : slices(std::move(seq)) {}

void
Ice::SlicedData::clear()
Expand All @@ -29,7 +29,7 @@ Ice::SlicedData::clear()
}
}

Ice::UnknownSlicedValue::UnknownSlicedValue(const string& unknownTypeId) : _unknownTypeId(unknownTypeId) {}
Ice::UnknownSlicedValue::UnknownSlicedValue(string unknownTypeId) noexcept : _unknownTypeId(std::move(unknownTypeId)) {}

const char*
Ice::UnknownSlicedValue::ice_id() const noexcept
Expand Down
44 changes: 22 additions & 22 deletions php/src/Types.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,27 +368,23 @@ IcePHP::StreamUtil::getSlicedDataMember(zval* obj, ObjectMap* objectMap)
{
assert(Z_OBJCE_P(s) == _sliceInfoType);

Ice::SliceInfoPtr info = make_shared<Ice::SliceInfo>();

zval* typeId = zend_hash_str_find(Z_OBJPROP_P(s), "typeId", sizeof("typeId") - 1);
assert(Z_TYPE_P(typeId) == IS_INDIRECT);
typeId = Z_INDIRECT_P(typeId);
assert(typeId && Z_TYPE_P(typeId) == IS_STRING);
info->typeId = string(Z_STRVAL_P(typeId), Z_STRLEN_P(typeId));

zval* compactId = zend_hash_str_find(Z_OBJPROP_P(s), "compactId", sizeof("compactId") - 1);
assert(Z_TYPE_P(compactId) == IS_INDIRECT);
compactId = Z_INDIRECT_P(compactId);
assert(compactId && Z_TYPE_P(compactId) == IS_LONG);
info->compactId = static_cast<long>(Z_LVAL_P(compactId));

zval* bytes = zend_hash_str_find(Z_OBJPROP_P(s), "bytes", sizeof("bytes") - 1);
assert(Z_TYPE_P(bytes) == IS_INDIRECT);
bytes = Z_INDIRECT_P(bytes);
assert(bytes && Z_TYPE_P(bytes) == IS_ARRAY);
HashTable* barr = Z_ARRVAL_P(bytes);
zval* e;
info->bytes.resize(zend_hash_num_elements(barr));
vector<byte> byteVector(zend_hash_num_elements(barr));

#if defined(__clang__)
# pragma clang diagnostic push
Expand All @@ -399,13 +395,33 @@ IcePHP::StreamUtil::getSlicedDataMember(zval* obj, ObjectMap* objectMap)
{
long l = static_cast<long>(Z_LVAL_P(e));
assert(l >= 0 && l <= 255);
info->bytes[i++] = static_cast<byte>(l);
byteVector[i++] = static_cast<byte>(l);
}
ZEND_HASH_FOREACH_END();
#if defined(__clang__)
# pragma clang diagnostic pop
#endif

zval* hasOptionalMembers =
zend_hash_str_find(Z_OBJPROP_P(s), "hasOptionalMembers", sizeof("hasOptionalMembers") - 1);
assert(Z_TYPE_P(hasOptionalMembers) == IS_INDIRECT);
hasOptionalMembers = Z_INDIRECT_P(hasOptionalMembers);
assert(
hasOptionalMembers &&
(Z_TYPE_P(hasOptionalMembers) == IS_TRUE || Z_TYPE_P(hasOptionalMembers) == IS_FALSE));

zval* isLastSlice = zend_hash_str_find(Z_OBJPROP_P(s), "isLastSlice", sizeof("isLastSlice") - 1);
assert(Z_TYPE_P(isLastSlice) == IS_INDIRECT);
isLastSlice = Z_INDIRECT_P(isLastSlice);
assert(isLastSlice && (Z_TYPE_P(isLastSlice) == IS_TRUE || Z_TYPE_P(isLastSlice) == IS_FALSE));

auto info = make_shared<Ice::SliceInfo>(
string(Z_STRVAL_P(typeId), Z_STRLEN_P(typeId)),
static_cast<long>(Z_LVAL_P(compactId)),
std::move(byteVector),
Z_TYPE_P(hasOptionalMembers) == IS_TRUE,
Z_TYPE_P(isLastSlice) == IS_TRUE);

zval* instances = zend_hash_str_find(Z_OBJPROP_P(s), "instances", sizeof("instances") - 1);
assert(Z_TYPE_P(instances) == IS_INDIRECT);
instances = Z_INDIRECT_P(instances);
Expand Down Expand Up @@ -440,22 +456,6 @@ IcePHP::StreamUtil::getSlicedDataMember(zval* obj, ObjectMap* objectMap)
#if defined(__clang__)
# pragma clang diagnostic pop
#endif

zval* hasOptionalMembers =
zend_hash_str_find(Z_OBJPROP_P(s), "hasOptionalMembers", sizeof("hasOptionalMembers") - 1);
assert(Z_TYPE_P(hasOptionalMembers) == IS_INDIRECT);
hasOptionalMembers = Z_INDIRECT_P(hasOptionalMembers);
assert(
hasOptionalMembers &&
(Z_TYPE_P(hasOptionalMembers) == IS_TRUE || Z_TYPE_P(hasOptionalMembers) == IS_FALSE));
info->hasOptionalMembers = Z_TYPE_P(hasOptionalMembers) == IS_TRUE;

zval* isLastSlice = zend_hash_str_find(Z_OBJPROP_P(s), "isLastSlice", sizeof("isLastSlice") - 1);
assert(Z_TYPE_P(isLastSlice) == IS_INDIRECT);
isLastSlice = Z_INDIRECT_P(isLastSlice);
assert(isLastSlice && (Z_TYPE_P(isLastSlice) == IS_TRUE || Z_TYPE_P(isLastSlice) == IS_FALSE));
info->isLastSlice = Z_TYPE_P(isLastSlice) == IS_TRUE;

slices.push_back(info);
}
ZEND_HASH_FOREACH_END();
Expand Down
26 changes: 13 additions & 13 deletions python/modules/IcePy/Types.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,15 +463,11 @@ IcePy::StreamUtil::getSlicedDataMember(PyObject* obj, ObjectMap* objectMap)
PyObjectHandle s = PyTuple_GET_ITEM(sl.get(), i);
Py_INCREF(s.get());

Ice::SliceInfoPtr info = std::make_shared<Ice::SliceInfo>();

PyObjectHandle typeId = getAttr(s.get(), "typeId", false);
assert(typeId.get());
info->typeId = getString(typeId.get());

PyObjectHandle compactId = getAttr(s.get(), "compactId", false);
assert(compactId.get());
info->compactId = static_cast<int>(PyLong_AsLong(compactId.get()));

PyObjectHandle bytes = getAttr(s.get(), "bytes", false);
assert(bytes.get());
Expand All @@ -480,7 +476,19 @@ IcePy::StreamUtil::getSlicedDataMember(PyObject* obj, ObjectMap* objectMap)
assert(PyBytes_Check(bytes.get()));
PyBytes_AsStringAndSize(bytes.get(), &str, &strsz);
vector<byte> vtmp(reinterpret_cast<byte*>(str), reinterpret_cast<byte*>(str + strsz));
info->bytes.swap(vtmp);

PyObjectHandle hasOptionalMembers = getAttr(s.get(), "hasOptionalMembers", false);
assert(hasOptionalMembers.get());

PyObjectHandle isLastSlice = getAttr(s.get(), "isLastSlice", false);
assert(isLastSlice.get());

auto info = std::make_shared<Ice::SliceInfo>(
getString(typeId.get()),
static_cast<int>(PyLong_AsLong(compactId.get())),
std::move(vtmp),
PyObject_IsTrue(hasOptionalMembers.get()) ? true : false,
PyObject_IsTrue(isLastSlice.get()) ? true : false);

PyObjectHandle instances = getAttr(s.get(), "instances", false);
assert(instances.get());
Expand All @@ -506,14 +514,6 @@ IcePy::StreamUtil::getSlicedDataMember(PyObject* obj, ObjectMap* objectMap)
info->instances.push_back(writer);
}

PyObjectHandle hasOptionalMembers = getAttr(s.get(), "hasOptionalMembers", false);
assert(hasOptionalMembers.get());
info->hasOptionalMembers = PyObject_IsTrue(hasOptionalMembers.get()) ? true : false;

PyObjectHandle isLastSlice = getAttr(s.get(), "isLastSlice", false);
assert(isLastSlice.get());
info->isLastSlice = PyObject_IsTrue(isLastSlice.get()) ? true : false;

slices.push_back(info);
}

Expand Down
26 changes: 10 additions & 16 deletions ruby/src/IceRuby/Types.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -336,23 +336,23 @@ IceRuby::StreamUtil::getSlicedDataMember(VALUE obj, ValueMap* valueMap)
{
volatile VALUE s = RARRAY_AREF(sl, i);

Ice::SliceInfoPtr info = std::make_shared<Ice::SliceInfo>();

volatile VALUE typeId = callRuby(rb_iv_get, s, "@typeId");
info->typeId = getString(typeId);

volatile VALUE compactId = callRuby(rb_iv_get, s, "@compactId");
info->compactId = static_cast<int32_t>(getInteger(compactId));

volatile VALUE bytes = callRuby(rb_iv_get, s, "@bytes");
assert(TYPE(bytes) == T_STRING);
const char* str = RSTRING_PTR(bytes);
const long len = RSTRING_LEN(bytes);
if (str != 0 && len != 0)
{
vector<byte> vtmp(reinterpret_cast<const byte*>(str), reinterpret_cast<const byte*>(str + len));
info->bytes.swap(vtmp);
}
vector<byte> vtmp(reinterpret_cast<const byte*>(str), reinterpret_cast<const byte*>(str + len));
volatile VALUE hasOptionalMembers = callRuby(rb_iv_get, s, "@hasOptionalMembers");
volatile VALUE isLastSlice = callRuby(rb_iv_get, s, "@isLastSlice");

auto info = std::make_shared<Ice::SliceInfo>(
getString(typeId),
static_cast<int32_t>(getInteger(compactId)),
std::move(vtmp),
hasOptionalMembers == Qtrue,
isLastSlice == Qtrue);

volatile VALUE instances = callRuby(rb_iv_get, s, "@instances");
assert(TYPE(instances) == T_ARRAY);
Expand All @@ -377,12 +377,6 @@ IceRuby::StreamUtil::getSlicedDataMember(VALUE obj, ValueMap* valueMap)
info->instances.push_back(writer);
}

volatile VALUE hasOptionalMembers = callRuby(rb_iv_get, s, "@hasOptionalMembers");
info->hasOptionalMembers = hasOptionalMembers == Qtrue;

volatile VALUE isLastSlice = callRuby(rb_iv_get, s, "@isLastSlice");
info->isLastSlice = isLastSlice == Qtrue;

slices.push_back(info);
}

Expand Down

0 comments on commit dc2e185

Please sign in to comment.