Skip to content

Commit

Permalink
Move TypeId to experimental
Browse files Browse the repository at this point in the history
TypeId is not quite ready yet. In particular TypeId instances created
for the same type in different components are not "equal".

This patch adds a test that will fail (thus DISABLED for now) and moves
the TypeId class to experimental to prevent people for using it for now.

Bug: 906125
Change-Id: I10f7707d70a66173676a50d7cf3d3a25f431f905
Reviewed-on: https://chromium-review.googlesource.com/c/1377738
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616787}
  • Loading branch information
carlscabgro authored and Commit Bot committed Dec 14, 2018
1 parent e96adaf commit 8a3962d
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 35 deletions.
1 change: 1 addition & 0 deletions base/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2603,6 +2603,7 @@ test("base_unittests") {
"//base/test:native_library_test_utils",
"//base/test:run_all_base_unittests",
"//base/test:test_support",
"//base/test:test_support_component",
"//base/third_party/dynamic_annotations",
"//testing/gmock",
"//testing/gtest",
Expand Down
22 changes: 18 additions & 4 deletions base/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,6 @@ static_library("test_support") {
"trace_event_analyzer.h",
"trace_to_file.cc",
"trace_to_file.h",
"type_id_test_support_a.cc",
"type_id_test_support_a.h",
"type_id_test_support_b.cc",
"type_id_test_support_b.h",
"values_test_util.cc",
"values_test_util.h",
]
Expand Down Expand Up @@ -277,6 +273,24 @@ static_library("test_support") {
}
}

component("test_support_component") {
testonly = true
sources = [
"type_id_test_support_a.cc",
"type_id_test_support_a.h",
"type_id_test_support_b.cc",
"type_id_test_support_b.h",
]
deps = [
"//base",
]
configs += [ ":base_test_implementation" ]
}

config("base_test_implementation") {
defines = [ "IS_BASE_TEST_IMPL" ]
}

config("perf_test_config") {
defines = [ "PERF_TEST" ]
}
Expand Down
9 changes: 5 additions & 4 deletions base/test/type_id_test_support_a.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ struct TypeInAnonymousNameSpace {};
namespace base {

// static
TypeId TypeIdTestSupportA::GetTypeIdForTypeInAnonymousNameSpace() {
return TypeId::Create<TypeInAnonymousNameSpace>();
experimental::TypeId
TypeIdTestSupportA::GetTypeIdForTypeInAnonymousNameSpace() {
return experimental::TypeId::Create<TypeInAnonymousNameSpace>();
}

TypeId TypeIdTestSupportA::GetTypeIdForUniquePtrInt() {
return TypeId::Create<std::unique_ptr<int>>();
experimental::TypeId TypeIdTestSupportA::GetTypeIdForUniquePtrInt() {
return experimental::TypeId::Create<std::unique_ptr<int>>();
}

} // namespace base
15 changes: 4 additions & 11 deletions base/test/type_id_test_support_a.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,15 @@
#ifndef BASE_TEST_TYPE_ID_TEST_SUPPORT_A_H_
#define BASE_TEST_TYPE_ID_TEST_SUPPORT_A_H_

#include "base/component_export.h"
#include "base/type_id.h"

namespace base {

// BASE_EXPORT depends on COMPONENT_BUILD.
// This will always be a separate shared library, so don't use BASE_EXPORT here.
#if defined(WIN32)
#define TEST_SUPPORT_EXPORT __declspec(dllexport)
#else
#define TEST_SUPPORT_EXPORT __attribute__((visibility("default")))
#endif // defined(WIN32)

// This is here to help test base::TypeId.
struct TEST_SUPPORT_EXPORT TypeIdTestSupportA {
static TypeId GetTypeIdForTypeInAnonymousNameSpace();
static TypeId GetTypeIdForUniquePtrInt();
struct COMPONENT_EXPORT(BASE_TEST) TypeIdTestSupportA {
static experimental::TypeId GetTypeIdForTypeInAnonymousNameSpace();
static experimental::TypeId GetTypeIdForUniquePtrInt();
};

} // namespace base
Expand Down
9 changes: 5 additions & 4 deletions base/test/type_id_test_support_b.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ struct TypeInAnonymousNameSpace {};
namespace base {

// static
TypeId TypeIdTestSupportB::GetTypeIdForTypeInAnonymousNameSpace() {
return TypeId::Create<TypeInAnonymousNameSpace>();
experimental::TypeId
TypeIdTestSupportB::GetTypeIdForTypeInAnonymousNameSpace() {
return experimental::TypeId::Create<TypeInAnonymousNameSpace>();
}

TypeId TypeIdTestSupportB::GetTypeIdForUniquePtrInt() {
return TypeId::Create<std::unique_ptr<int>>();
experimental::TypeId TypeIdTestSupportB::GetTypeIdForUniquePtrInt() {
return experimental::TypeId::Create<std::unique_ptr<int>>();
}

} // namespace base
15 changes: 4 additions & 11 deletions base/test/type_id_test_support_b.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,15 @@
#ifndef BASE_TEST_TYPE_ID_TEST_SUPPORT_B_H_
#define BASE_TEST_TYPE_ID_TEST_SUPPORT_B_H_

#include "base/component_export.h"
#include "base/type_id.h"

namespace base {

// BASE_EXPORT depends on COMPONENT_BUILD.
// This will always be a separate shared library, so don't use BASE_EXPORT here.
#if defined(WIN32)
#define TEST_SUPPORT_EXPORT __declspec(dllexport)
#else
#define TEST_SUPPORT_EXPORT __attribute__((visibility("default")))
#endif // defined(WIN32)

// This is here to help test base::TypeId.
struct TEST_SUPPORT_EXPORT TypeIdTestSupportB {
static TypeId GetTypeIdForTypeInAnonymousNameSpace();
static TypeId GetTypeIdForUniquePtrInt();
struct COMPONENT_EXPORT(BASE_TEST) TypeIdTestSupportB {
static experimental::TypeId GetTypeIdForTypeInAnonymousNameSpace();
static experimental::TypeId GetTypeIdForUniquePtrInt();
};

} // namespace base
Expand Down
2 changes: 2 additions & 0 deletions base/type_id.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "base/strings/string_number_conversions.h"

namespace base {
namespace experimental {

std::string TypeId::ToString() const {
#if DCHECK_IS_ON()
Expand All @@ -20,4 +21,5 @@ std::ostream& operator<<(std::ostream& out, const TypeId& type_id) {
return out << type_id.ToString();
}

} // namespace experimental
} // namespace base
3 changes: 3 additions & 0 deletions base/type_id.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#include "build/build_config.h"

namespace base {
// Not ready for public consumption yet.
namespace experimental {

// A substitute for RTTI that uses the linker to uniquely reserve an address in
// the binary for each type.
Expand Down Expand Up @@ -61,6 +63,7 @@ constexpr char TypeId::TypeTag<Type>::dummy_var;

BASE_EXPORT std::ostream& operator<<(std::ostream& out, const TypeId& type_id);

} // namespace experimental
} // namespace base

#endif // BASE_TYPE_ID_H_
10 changes: 9 additions & 1 deletion base/type_id_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "testing/gtest/include/gtest/gtest.h"

namespace base {

namespace experimental {
namespace {

struct T {};
Expand Down Expand Up @@ -85,4 +85,12 @@ TEST(TypeId, IdenticalTypesFromDifferentCompilationUnitsMatch) {
TypeIdTestSupportB::GetTypeIdForUniquePtrInt());
}

TEST(TypeId, DISABLED_IdenticalTypesFromComponentAndStaticLibrary) {
// Code generated for the test itself is statically linked. Make sure it works
// with components
constexpr TypeId static_linked_type = TypeId::Create<std::unique_ptr<int>>();
EXPECT_EQ(static_linked_type, TypeIdTestSupportA::GetTypeIdForUniquePtrInt());
}

} // namespace experimental
} // namespace base

0 comments on commit 8a3962d

Please sign in to comment.