Skip to content

Commit

Permalink
Introduce a more generic version of IdType for opaque aliases
Browse files Browse the repository at this point in the history 
This one works with strings and other complex types in addition to
integrals.

Bug: 954080
Change-Id: Ic9961e0ccffdce89d160a5b4d8cab9b7d94a100b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1617481
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Maciej Pawlowski <mpawlowski@opera.com>
Cr-Commit-Position: refs/heads/master@{#662532}
  • Loading branch information
@maciej-pawlowski-opera
maciej-pawlowski-opera authored and Commit Bot committed May 23, 2019
1 parent 71e356e commit c00800e
Show file tree
Hide file tree
Showing 6 changed files with 425 additions and 201 deletions.
2 changes: 2 additions & 0 deletions base/util/type_safety/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ import("//build/config/jumbo.gni")
source_set("type_safety") {
sources = [
"id_type.h",
"strong_alias.h",
]
}

source_set("tests") {
testonly = true
sources = [
"id_type_unittest.cc",
"strong_alias_unittest.cc",
]

deps = [
Expand Down
83 changes: 25 additions & 58 deletions base/util/type_safety/id_type.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
#ifndef BASE_UTIL_TYPE_SAFETY_ID_TYPE_H_
#define BASE_UTIL_TYPE_SAFETY_ID_TYPE_H_

#include <stdint.h>
#include <cstddef>
#include <ostream>
#include <type_traits>
#include <cstdint>

#include "base/util/type_safety/strong_alias.h"

namespace util {

// A specialization of StrongAlias for integer-based identifiers.
//
// IdType32<>, IdType64<>, etc. wrap an integer id in a custom, type-safe type.
//
// IdType32<Foo> is an alternative to int, for a class Foo with methods like:
Expand Down Expand Up @@ -38,72 +41,36 @@
// - it can be used in IPC messages.
//
// IdType32<Foo> has the following differences from a bare int32_t:
// - it forces coercions to go through GetUnsafeValue and FromUnsafeValue;
// - it forces coercions to go through the explicit constructor and value()
// getter;
// - it restricts the set of available operations (i.e. no multiplication);
// - it ensures initialization to zero and allows checking against
// default-initialized values via is_null method.

namespace util {

// - it default-constructs to a null value and allows checking against the null
// value via is_null method.
template <typename TypeMarker, typename WrappedType, WrappedType kInvalidValue>
class IdType {
class IdType : public StrongAlias<TypeMarker, WrappedType> {
public:
IdType() : value_(kInvalidValue) {}
bool is_null() const { return value_ == kInvalidValue; }

static IdType FromUnsafeValue(WrappedType value) { return IdType(value); }
WrappedType GetUnsafeValue() const { return value_; }

IdType(const IdType& other) = default;
IdType& operator=(const IdType& other) = default;

bool operator==(const IdType& other) const { return value_ == other.value_; }
bool operator!=(const IdType& other) const { return value_ != other.value_; }
bool operator<(const IdType& other) const { return value_ < other.value_; }
bool operator<=(const IdType& other) const { return value_ <= other.value_; }

// Hasher to use in std::unordered_map, std::unordered_set, etc.
struct Hasher {
using argument_type = IdType;
using result_type = std::size_t;
result_type operator()(const argument_type& id) const {
return std::hash<WrappedType>()(id.GetUnsafeValue());
}
};
using StrongAlias<TypeMarker, WrappedType>::StrongAlias;
// Default-construct in the null state.
IdType() : StrongAlias<TypeMarker, WrappedType>::StrongAlias(kInvalidValue) {}

protected:
explicit IdType(WrappedType val) : value_(val) {}
bool is_null() const { return this->value() == kInvalidValue; }

private:
// In theory WrappedType could be any type that supports ==, <, <<, std::hash,
// etc., but to make things simpler (both for users and for maintainers) we
// explicitly restrict the design space to integers. This means the users
// can safely assume that IdType is relatively small and cheap to copy
// and the maintainers don't have to worry about WrappedType being a complex
// type (i.e. std::string or std::pair or a move-only type).
using IntegralWrappedType =
typename std::enable_if<std::is_integral<WrappedType>::value,
WrappedType>::type;
IntegralWrappedType value_;
// TODO(mpawlowski) Replace these with constructor/value() getter. The
// conversions are safe as long as they're explicit (which is taken care of by
// StrongAlias).
static IdType FromUnsafeValue(WrappedType value) { return IdType(value); }
WrappedType GetUnsafeValue() const { return this->value(); }
};

// Type aliases for convenience:
template <typename TypeMarker>
using IdType32 = IdType<TypeMarker, int32_t, 0>;
using IdType32 = IdType<TypeMarker, std::int32_t, 0>;
template <typename TypeMarker>
using IdTypeU32 = IdType<TypeMarker, uint32_t, 0>;
using IdTypeU32 = IdType<TypeMarker, std::uint32_t, 0>;
template <typename TypeMarker>
using IdType64 = IdType<TypeMarker, int64_t, 0>;
using IdType64 = IdType<TypeMarker, std::int64_t, 0>;
template <typename TypeMarker>
using IdTypeU64 = IdType<TypeMarker, uint64_t, 0>;

template <typename TypeMarker, typename WrappedType, WrappedType kInvalidValue>
std::ostream& operator<<(
std::ostream& stream,
const IdType<TypeMarker, WrappedType, kInvalidValue>& id) {
return stream << id.GetUnsafeValue();
}

using IdTypeU64 = IdType<TypeMarker, std::uint64_t, 0>;
} // namespace util

#endif // BASE_UTIL_TYPE_SAFETY_ID_TYPE_H_
143 changes: 0 additions & 143 deletions base/util/type_safety/id_type_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@
// found in the LICENSE file.

#include <limits>
#include <map>
#include <sstream>
#include <string>
#include <type_traits>
#include <unordered_map>

#include "base/util/type_safety/id_type.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand All @@ -19,16 +14,6 @@ namespace {
class Foo;
using FooId = IdType<Foo, int, 0>;

class Bar;
using BarId = IdType<Bar, int, 0>;

class AnotherIdMarker;
class DerivedId : public IdType<AnotherIdMarker, int, 0> {
public:
explicit DerivedId(int unsafe_value)
: IdType<AnotherIdMarker, int, 0>(unsafe_value) {}
};

} // namespace

TEST(IdType, DefaultValueIsInvalid) {
Expand All @@ -41,92 +26,6 @@ TEST(IdType, NormalValueIsValid) {
EXPECT_FALSE(foo_id.is_null());
}

TEST(IdType, OutputStreamTest) {
FooId foo_id = FooId::FromUnsafeValue(123);

std::ostringstream ss;
ss << foo_id;
EXPECT_EQ("123", ss.str());
}

TEST(IdType, IdType32) {
IdType32<Foo> id;

EXPECT_EQ(0, id.GetUnsafeValue());
static_assert(sizeof(int32_t) == sizeof(id), "");
}

TEST(IdType, IdTypeU32) {
IdTypeU32<Foo> id;

EXPECT_EQ(0u, id.GetUnsafeValue());
static_assert(sizeof(uint32_t) == sizeof(id), "");
}

TEST(IdType, IdType64) {
IdType64<Foo> id;

EXPECT_EQ(0, id.GetUnsafeValue());
static_assert(sizeof(int64_t) == sizeof(id), "");
}

TEST(IdType, IdTypeU64) {
IdTypeU64<Foo> id;

EXPECT_EQ(0u, id.GetUnsafeValue());
static_assert(sizeof(uint64_t) == sizeof(id), "");
}

TEST(IdType, DerivedClasses) {
DerivedId derived_id(456);

std::ostringstream ss;
ss << derived_id;
EXPECT_EQ("456", ss.str());

std::map<DerivedId, std::string> ordered_map;
ordered_map[derived_id] = "blah";
EXPECT_EQ(ordered_map[derived_id], "blah");

std::unordered_map<DerivedId, std::string, DerivedId::Hasher> unordered_map;
unordered_map[derived_id] = "blah2";
EXPECT_EQ(unordered_map[derived_id], "blah2");
}

TEST(IdType, StaticAsserts) {
static_assert(!std::is_constructible<FooId, int>::value,
"Should be impossible to construct FooId from a raw integer.");
static_assert(!std::is_convertible<int, FooId>::value,
"Should be impossible to convert a raw integer into FooId.");

static_assert(!std::is_constructible<FooId, BarId>::value,
"Should be impossible to construct FooId from a BarId.");
static_assert(!std::is_convertible<BarId, FooId>::value,
"Should be impossible to convert a BarId into FooId.");

// The presence of a custom default constructor means that FooId is not a
// "trivial" class and therefore is not a POD type (unlike an int32_t).
// At the same time FooId has almost all of the properties of a POD type:
// - is "trivially copyable" (i.e. is memcpy-able),
// - has "standard layout" (i.e. interops with things expecting C layout).
// See http://stackoverflow.com/a/7189821 for more info about these
// concepts.
static_assert(std::is_standard_layout<FooId>::value,
"FooId should have standard layout. "
"See http://stackoverflow.com/a/7189821 for more info.");
static_assert(sizeof(FooId) == sizeof(int),
"FooId should be the same size as the raw integer it wraps.");
// TODO(lukasza): Enable these once <type_traits> supports all the standard
// C++11 equivalents (i.e. std::is_trivially_copyable instead of the
// non-standard std::has_trivial_copy_assign).
// static_assert(std::has_trivial_copy_constructor<FooId>::value,
// "FooId should have a trivial copy constructor.");
// static_assert(std::has_trivial_copy_assign<FooId>::value,
// "FooId should have a trivial copy assignment operator.");
// static_assert(std::has_trivial_destructor<FooId>::value,
// "FooId should have a trivial destructor.");
}

class IdTypeSpecificValueTest : public ::testing::TestWithParam<int> {
protected:
FooId test_id() { return FooId::FromUnsafeValue(GetParam()); }
Expand All @@ -139,55 +38,13 @@ class IdTypeSpecificValueTest : public ::testing::TestWithParam<int> {
}
};

TEST_P(IdTypeSpecificValueTest, ComparisonToSelf) {
EXPECT_TRUE(test_id() == test_id());
EXPECT_FALSE(test_id() != test_id());
EXPECT_FALSE(test_id() < test_id());
}

TEST_P(IdTypeSpecificValueTest, ComparisonToOther) {
EXPECT_FALSE(test_id() == other_id());
EXPECT_TRUE(test_id() != other_id());
}

TEST_P(IdTypeSpecificValueTest, UnsafeValueRoundtrips) {
int original_value = GetParam();
FooId id = FooId::FromUnsafeValue(original_value);
int final_value = id.GetUnsafeValue();
EXPECT_EQ(original_value, final_value);
}

TEST_P(IdTypeSpecificValueTest, Copying) {
FooId original = test_id();

FooId copy_via_constructor(original);
EXPECT_EQ(original, copy_via_constructor);

FooId copy_via_assignment;
copy_via_assignment = original;
EXPECT_EQ(original, copy_via_assignment);
}

TEST_P(IdTypeSpecificValueTest, StdUnorderedMap) {
std::unordered_map<FooId, std::string, FooId::Hasher> map;

map[test_id()] = "test_id";
map[other_id()] = "other_id";

EXPECT_EQ(map[test_id()], "test_id");
EXPECT_EQ(map[other_id()], "other_id");
}

TEST_P(IdTypeSpecificValueTest, StdMap) {
std::map<FooId, std::string> map;

map[test_id()] = "test_id";
map[other_id()] = "other_id";

EXPECT_EQ(map[test_id()], "test_id");
EXPECT_EQ(map[other_id()], "other_id");
}

INSTANTIATE_TEST_SUITE_P(,
IdTypeSpecificValueTest,
::testing::Values(std::numeric_limits<int>::min(),
Expand Down
Loading

0 comments on commit c00800e

Please sign in to comment.