Skip to content

GitLab

T
tangled
Commit 8bbdf184 authored by Takumi Fujimoto's avatar Takumi Fujimoto Committed by Commit Bot
Browse files
Options

Revert "Implement initializer list support for containers" 

This reverts commit 01efb7a7.

Reason for revert: Causing bot failures:
https://ci.chromium.org/p/chromium/builders/ci/Win7%20%2832%29%20Tests

Original change's description:
> Implement initializer list support for containers
> 
> Implement initializer list support for creating arrays and dictionaries
> in TracedValue::Build. Thus speed up adding debugging tracing of
> multiple parameters, including container types.
> 
> Follow-up of: crrev.com/c/2263160
> 
> Bug: 1043616
> Change-Id: I2d3205d550e6ce6328ea253e6b94f9cc2cbe3098
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346286
> Reviewed-by: Eric Seckler <eseckler@chromium.org>
> Reviewed-by: Alexander Timin <altimin@chromium.org>
> Commit-Queue: Karel Král <karelkral@google.com>
> Cr-Commit-Position: refs/heads/master@{#797706}

TBR=altimin@chromium.org,eseckler@chromium.org,karelkral@google.com

Change-Id: I7334c762ede0d06226398ee9ff7b38bf5a30e44e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1043616
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354830Reviewed-by: default avatarTakumi Fujimoto <takumif@chromium.org>
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797787}
parent 8f6a6e3b
Hide whitespace changes
Inline Side-by-side
Showing with 55 additions and 317 deletions
base/trace_event/traced_value.cc
View file @ 8bbdf184
......@@ -597,11 +597,6 @@ void TracedValue::AppendString(base::StringPiece value) {
writer_->AppendString(value);
}
void TracedValue::AppendPointer(void* value) {
DCHECK_CURRENT_CONTAINER_IS(kStackTypeArray);
writer_->AppendString(PointerToString(value));
}
void TracedValue::BeginArray() {
DCHECK_CURRENT_CONTAINER_IS(kStackTypeArray);
DEBUG_PUSH_CONTAINER(kStackTypeArray);
......@@ -647,202 +642,71 @@ void TracedValue::EstimateTraceMemoryOverhead(
writer_->EstimateTraceMemoryOverhead(overhead);
}
TracedValue::Array::Array(const std::initializer_list<ArrayItem> items) {
items_ = std::move(items);
}
TracedValue::Array::Array(TracedValue::Array&& other) {
items_ = std::move(other.items_);
}
void TracedValue::Array::WriteToValue(TracedValue* value) const {
for (const auto& item : items_) {
item.WriteToValue(value);
}
}
TracedValue::Dictionary::Dictionary(
const std::initializer_list<DictionaryItem> items) {
items_ = items;
}
TracedValue::Dictionary::Dictionary(TracedValue::Dictionary&& other) {
items_ = std::move(other.items_);
}
void TracedValue::Dictionary::WriteToValue(TracedValue* value) const {
for (const auto& item : items_) {
item.WriteToValue(value);
}
}
TracedValue::ValueHolder::ValueHolder(int value) {
TracedValue::DictionaryItem::DictionaryItem(const char* name, int value) {
name_ = name;
kept_value_.int_value = value;
kept_value_type_ = KeptValueType::kIntType;
}
TracedValue::ValueHolder::ValueHolder(double value) {
TracedValue::DictionaryItem::DictionaryItem(const char* name, double value) {
name_ = name;
kept_value_.double_value = value;
kept_value_type_ = KeptValueType::kDoubleType;
}
TracedValue::ValueHolder::ValueHolder(bool value) {
TracedValue::DictionaryItem::DictionaryItem(const char* name, bool value) {
name_ = name;
kept_value_.bool_value = value;
kept_value_type_ = KeptValueType::kBoolType;
}
TracedValue::ValueHolder::ValueHolder(base::StringPiece value) {
kept_value_.string_piece_value = value;
kept_value_type_ = KeptValueType::kStringPieceType;
}
TracedValue::ValueHolder::ValueHolder(const std::string& value) {
TracedValue::DictionaryItem::DictionaryItem(const char* name,
base::StringPiece value) {
name_ = name;
kept_value_.string_piece_value = value;
kept_value_type_ = KeptValueType::kStringPieceType;
}
TracedValue::ValueHolder::ValueHolder(void* value) {
TracedValue::DictionaryItem::DictionaryItem(const char* name, void* value) {
name_ = name;
kept_value_.void_ptr_value = value;
kept_value_type_ = KeptValueType::kVoidPtrType;
}
TracedValue::ValueHolder::ValueHolder(const char* value) {
TracedValue::DictionaryItem::DictionaryItem(const char* name,
const char* value) {
name_ = name;
kept_value_.string_piece_value = value;
kept_value_type_ = KeptValueType::kStringPieceType;
}
TracedValue::ValueHolder::ValueHolder(TracedValue::Dictionary& value) {
new (&kept_value_.dictionary_value) TracedValue::Dictionary(std::move(value));
kept_value_type_ = KeptValueType::kDictionaryType;
}
TracedValue::ValueHolder::ValueHolder(TracedValue::Array& value) {
new (&kept_value_.array_value) TracedValue::Array(std::move(value));
kept_value_type_ = KeptValueType::kArrayType;
}
TracedValue::ValueHolder::ValueHolder(TracedValue::ValueHolder&& other) {
// Remember to call a destructor if ever necessary.
switch (other.kept_value_type_) {
case KeptValueType::kIntType: {
kept_value_.int_value = other.kept_value_.int_value;
break;
}
case KeptValueType::kDoubleType: {
kept_value_.double_value = other.kept_value_.double_value;
break;
}
case KeptValueType::kBoolType: {
kept_value_.bool_value = other.kept_value_.bool_value;
break;
}
case KeptValueType::kStringPieceType: {
kept_value_.string_piece_value = other.kept_value_.string_piece_value;
break;
}
case KeptValueType::kVoidPtrType: {
kept_value_.void_ptr_value = other.kept_value_.void_ptr_value;
break;
}
case KeptValueType::kArrayType: {
new (&kept_value_.array_value)
TracedValue::Array(std::move(other.kept_value_.array_value));
break;
}
case KeptValueType::kDictionaryType: {
new (&kept_value_.dictionary_value) TracedValue::Dictionary(
std::move(other.kept_value_.dictionary_value));
break;
}
}
kept_value_type_ = other.kept_value_type_;
}
void TracedValue::ValueHolder::WriteToValue(TracedValue* value) const {
switch (kept_value_type_) {
case KeptValueType::kIntType: {
value->AppendInteger(kept_value_.int_value);
break;
}
case KeptValueType::kDoubleType: {
value->AppendDouble(kept_value_.double_value);
break;
}
case KeptValueType::kBoolType: {
value->AppendBoolean(kept_value_.bool_value);
break;
}
case KeptValueType::kStringPieceType: {
value->AppendString(kept_value_.string_piece_value);
break;
}
case KeptValueType::kVoidPtrType: {
value->AppendPointer(kept_value_.void_ptr_value);
break;
}
case KeptValueType::kArrayType: {
value->BeginArray();
kept_value_.array_value.WriteToValue(value);
value->EndArray();
break;
}
case KeptValueType::kDictionaryType: {
value->BeginDictionary();
kept_value_.dictionary_value.WriteToValue(value);
value->EndDictionary();
break;
}
}
}
void TracedValue::ValueHolder::WriteToValue(const char* name,
TracedValue* value) const {
void TracedValue::DictionaryItem::WriteToValue(TracedValue* value) const {
switch (kept_value_type_) {
case KeptValueType::kIntType: {
value->SetInteger(name, kept_value_.int_value);
value->SetInteger(name_, kept_value_.int_value);
break;
}
case KeptValueType::kDoubleType: {
value->SetDouble(name, kept_value_.double_value);
value->SetDouble(name_, kept_value_.double_value);
break;
}
case KeptValueType::kBoolType: {
value->SetBoolean(name, kept_value_.bool_value);
value->SetBoolean(name_, kept_value_.bool_value);
break;
}
case KeptValueType::kStringPieceType: {
value->SetString(name, kept_value_.string_piece_value);
value->SetString(name_, kept_value_.string_piece_value);
break;
}
case KeptValueType::kVoidPtrType: {
value->SetPointer(name, kept_value_.void_ptr_value);
break;
}
case KeptValueType::kArrayType: {
value->BeginArray(name);
kept_value_.array_value.WriteToValue(value);
value->EndArray();
break;
}
case KeptValueType::kDictionaryType: {
value->BeginDictionary(name);
kept_value_.dictionary_value.WriteToValue(value);
value->EndDictionary();
value->SetPointer(name_, kept_value_.void_ptr_value);
break;
}
}
}
void TracedValue::ArrayItem::WriteToValue(TracedValue* value) const {
ValueHolder::WriteToValue(value);
}
void TracedValue::DictionaryItem::WriteToValue(TracedValue* value) const {
ValueHolder::WriteToValue(name_, value);
}
std::unique_ptr<TracedValue> TracedValue::Build(
const std::initializer_list<DictionaryItem> items) {
std::initializer_list<DictionaryItem> items) {
std::unique_ptr<TracedValue> value(new TracedValue());
for (const auto& item : items) {
item.WriteToValue(value.get());
......
base/trace_event/traced_value.h
View file @ 8bbdf184
......@@ -57,7 +57,6 @@ class BASE_EXPORT TracedValue : public ConvertableToTraceFormat {
void AppendDouble(double);
void AppendBoolean(bool);
void AppendString(base::StringPiece);
void AppendPointer(void*);
void BeginArray();
void BeginDictionary();
......@@ -67,116 +66,57 @@ class BASE_EXPORT TracedValue : public ConvertableToTraceFormat {
void EstimateTraceMemoryOverhead(TraceEventMemoryOverhead* overhead) override;
class BASE_EXPORT Array;
class BASE_EXPORT Dictionary;
class BASE_EXPORT ValueHolder;
class BASE_EXPORT ArrayItem;
// TracedValue::Build is a friend of TracedValue::DictionaryItem.
class BASE_EXPORT DictionaryItem;
// Helper to enable easier initialization of |TracedValue|. This is intended
// for quick local debugging as there is overhead of creating
// |std::initializer_list| of name-value objects (in the case of containers
// the value is also a |std::initializer_list|). Generally the helper types
// |TracedValue::Dictionary|, |TracedValue::Array|,
// |TracedValue::DictionaryItem|, |TracedValue::ArrayItem| must be valid as
// well as their internals (e.g., |base::StringPiece| data should be valid
// when |TracedValue::Build| is called; |TracedValue::Array| or
// |TracedValue::Dictionary| holds a |std::initializer_list| whose underlying
// array needs to be valid when calling |TracedValue::Build|).
// Helper to enable easier initialization of TracedValue. This is intended for
// quick local debugging as there is overhead of creating
// std::initializer_list of name-value objects. This method does not support
// creation of dictionaries or arrays.
//
// Example:
// auto value = TracedValue::Build({
// {"int_var_name", 42},
// {"double_var_name", 3.14},
// {"string_var_name", "hello world"},
// {"empty_array", TracedValue::Array({})},
// {"dictionary", TracedValue::Dictionary({
// {"my_ptr", static_cast<void*>(my_ptr)},
// {"nested_array", TracedValue::Array({1, false, 0.5})},
// })},
// {"string_var_name", "hello world"}
// });
static std::unique_ptr<TracedValue> Build(
const std::initializer_list<DictionaryItem> items);
// An |Array| instance represents an array of |ArrayItem| objects. This is a
// helper to allow initializer list like construction of arrays using
// |TracedValue::Build|.
//
// An instance holds an |std::initializer_list<TracedValue::ArrayItem>| and is
// cheap to copy (copying the initializer_list does not copy the underlying
// objects). The underlying array must exist at the time when
// |TracedValue::Build| is called.
class Array {
public:
// This constructor expects that the initializer_list is valid when
// |TracedValue::Build| is called.
Array(const std::initializer_list<ArrayItem> items);
Array(Array&&);
void WriteToValue(TracedValue* value) const;
private:
std::initializer_list<ArrayItem> items_;
};
// A helper to hold a dictionary. Similar to |TracedValue::Array|.
class Dictionary {
public:
// This constructor expects that the initializer_list is valid when
// |TracedValue::Build| is called.
Dictionary(const std::initializer_list<DictionaryItem> items);
Dictionary(Dictionary&&);
void WriteToValue(TracedValue* value) const;
private:
std::initializer_list<DictionaryItem> items_;
};
// |name| is assumed to be a long lived "quoted" string.
static std::unique_ptr<TracedValue> Build(
std::initializer_list<DictionaryItem> items);
// A |ValueHolder| holds a single value or a container (int, double... or an
// |Array| / |Dictionary|). Not to be used outside of the context of
// |TracedValue::Build| (has one parameter implicit constructors).
//
// Base class for |TracedValue::ArrayItem| and |TracedValue::DictionaryItem|.
class ValueHolder {
// DictionaryItem instance represents a single name-value pair.
class DictionaryItem {
public:
// Implicit constructors allow constructing |DictionaryItem| without having
// to write |{"name", TracedValue::ValueHolder(1)}|.
ValueHolder(int value); // NOLINT(google-explicit-constructor)
ValueHolder(double value); // NOLINT(google-explicit-constructor)
ValueHolder(bool value); // NOLINT(google-explicit-constructor)
ValueHolder(void* value); // NOLINT(google-explicit-constructor)
// These constructors assume that |name| is a long lived "quoted" string.
DictionaryItem(const char* name, int value);
DictionaryItem(const char* name, double value);
DictionaryItem(const char* name, bool value);
DictionaryItem(const char* name, void* value);
// StringPiece's backing storage / const char* pointer needs to remain valid
// until TracedValue::Build is called.
// NOLINTNEXTLINE(google-explicit-constructor)
ValueHolder(base::StringPiece value);
// NOLINTNEXTLINE(google-explicit-constructor)
ValueHolder(const std::string& value);
DictionaryItem(const char* name, base::StringPiece value);
// Define an explicit overload for const char* to resolve the ambiguity
// between the base::StringPiece, void*, and bool constructors for string
// literals.
ValueHolder(const char* value); // NOLINT(google-explicit-constructor)
ValueHolder(Array& value); // NOLINT(google-explicit-constructor)
ValueHolder(Dictionary& value); // NOLINT(google-explicit-constructor)
ValueHolder(ValueHolder&&);
DictionaryItem(const char* name, const char* value);
private:
friend std::unique_ptr<TracedValue> TracedValue::Build(
std::initializer_list<DictionaryItem> items);
protected:
void WriteToValue(TracedValue* value) const;
void WriteToValue(const char* name, TracedValue* value) const;
private:
union KeptValue {
// Copy is handled by the holder (based on
// |TracedValue::ValueHolder::kept_value_type_|).
int int_value;
double double_value;
bool bool_value;
base::StringPiece string_piece_value;
void* void_ptr_value;
Array array_value;
Dictionary dictionary_value;
// Default constructor is implicitly deleted because union field has a
// non-trivial default constructor.
KeptValue() {} // NOLINT(modernize-use-equals-default)
KeptValue() {}
};
// Reimplementing a subset of C++17 std::variant.
......@@ -186,41 +126,11 @@ class BASE_EXPORT TracedValue : public ConvertableToTraceFormat {
kBoolType,
kStringPieceType,
kVoidPtrType,
kArrayType,
kDictionaryType,
};
KeptValue kept_value_;
KeptValueType kept_value_type_;
};
// |ArrayItem| is a |ValueHolder| which can be used to construct an |Array|.
class ArrayItem : public ValueHolder {
public:
// Implicit constructors allow calling |TracedValue::Array({1, true, 3.14})|
// instead of |TracedValue::Array({TracedValue::ArrayItem(1),
// TracedValue::ArrayItem(true), TracedValue::ArrayItem(3.14)})|.
template <typename T>
// NOLINTNEXTLINE(google-explicit-constructor)
ArrayItem(T value) : ValueHolder(value) {}
void WriteToValue(TracedValue* value) const;
};
// |DictionaryItem| instance represents a single name-value pair.
//
// |name| is assumed to be a long lived "quoted" string.
class DictionaryItem : public ValueHolder {
public:
// These constructors assume that |name| is a long lived "quoted" string.
template <typename T>
DictionaryItem(const char* name, T value)
: ValueHolder(value), name_(name) {}
void WriteToValue(TracedValue* value) const;
private:
const char* name_;
KeptValueType kept_value_type_;
};
// A custom serialization class can be supplied by implementing the
......
base/trace_event/traced_value_unittest.cc
View file @ 8bbdf184
......@@ -15,31 +15,6 @@
namespace base {
namespace trace_event {
TEST(TraceEventArgumentTest, InitializerListCreatedContainers) {
std::string json;
TracedValue::Build(
{
{"empty_array", TracedValue::Array({})},
{"empty_dictionary", TracedValue::Dictionary({})},
{"nested_array", TracedValue::Array({
TracedValue::Array({}),
TracedValue::Dictionary({}),
true,
})},
{"nested_dictionary", TracedValue::Dictionary({
{"d", TracedValue::Dictionary({})},
{"a", TracedValue::Array({})},
{"b", true},
})},
})
->AppendAsTraceFormat(&json);
EXPECT_EQ(
"{\"empty_array\":[],\"empty_dictionary\":{},"
"\"nested_array\":[[],{},true],"
"\"nested_dictionary\":{\"d\":{},\"a\":[],\"b\":true}}",
json);
}
TEST(TraceEventArgumentTest, InitializerListCreatedFlatDictionary) {
std::string json;
TracedValue::Build({{"bool_var", true},
......@@ -53,34 +28,23 @@ TEST(TraceEventArgumentTest, InitializerListCreatedFlatDictionary) {
json);
}
std::string SayHello() {
// Create a string by concatenating two strings, so that there is no literal
// corresponding to the result.
return std::string("hello ") + std::string("world");
}
TEST(TraceEventArgumentTest, StringAndPointerConstructors) {
std::string json;
const char* const_char_ptr_var = "const char* value";
TracedValue::Build(
{
{"literal_var", "literal"},
{"std_string_var", std::string("std::string value")},
{"string_from_function", SayHello()},
{"string_from_lambda", []() { return std::string("hello"); }()},
{"base_string_piece_var",
base::StringPiece("base::StringPiece value")},
{"const_char_ptr_var", const_char_ptr_var},
{"void_nullptr", static_cast<void*>(nullptr)},
{"int_nullptr", static_cast<int*>(nullptr)},
{"void_1234ptr", reinterpret_cast<void*>(0x1234)},
})
TracedValue::Build({
{"literal_var", "literal"},
{"std_string_var", std::string("std::string value")},
{"base_string_piece_var",
base::StringPiece("base::StringPiece value")},
{"const_char_ptr_var", const_char_ptr_var},
{"void_nullptr", static_cast<void*>(nullptr)},
{"int_nullptr", static_cast<int*>(nullptr)},
{"void_1234ptr", reinterpret_cast<void*>(0x1234)},
})
->AppendAsTraceFormat(&json);
EXPECT_EQ(
"{\"literal_var\":\"literal\","
"\"std_string_var\":\"std::string value\","
"\"string_from_function\":\"hello world\","
"\"string_from_lambda\":\"hello\","
"\"base_string_piece_var\":\"base::StringPiece value\","
"\"const_char_ptr_var\":\"const char* value\","
"\"void_nullptr\":\"0x0\","
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment