Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2755)

Unified Diff: base/values.cc

Issue 2740373002: Cleaning up base::Value assignment. (Closed)
Patch Set: Always destroy in move. Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/values.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/values.cc
diff --git a/base/values.cc b/base/values.cc
index 5cc0d693bdd11c12e60f14f8921bec3cd303e4da..337c28f706493e85f2088ff67ee1754f8a0d280e 100644
--- a/base/values.cc
+++ b/base/values.cc
@@ -173,27 +173,21 @@ Value::Value(std::vector<char>&& in_blob) : type_(Type::BINARY) {
}
Value& Value::operator=(const Value& that) {
- if (this != &that) {
- if (type_ == that.type_) {
- InternalCopyAssignFromSameType(that);
- } else {
- InternalCleanup();
- InternalCopyConstructFrom(that);
- }
+ if (type_ == that.type_) {
+ InternalCopyAssignFromSameType(that);
+ } else {
+ // this is not a self assignment because the type_ doesn't match.
vabr (Chromium) 2017/03/13 10:56:01 this -> This
dyaroshev 2017/03/13 21:15:42 Done.
+ InternalCleanup();
+ InternalCopyConstructFrom(that);
}
return *this;
}
Value& Value::operator=(Value&& that) {
- if (this != &that) {
- if (type_ == that.type_) {
- InternalMoveAssignFromSameType(std::move(that));
- } else {
- InternalCleanup();
- InternalMoveConstructFrom(std::move(that));
- }
- }
+ DCHECK(this != &that) << " attempt to self move assign.";
dcheng 2017/03/11 06:30:07 Super minor nit: no space at the beginning of the
dyaroshev 2017/03/13 21:15:42 Done.
+ InternalCleanup();
+ InternalMoveConstructFrom(std::move(that));
return *this;
}
@@ -562,32 +556,6 @@ void Value::InternalCopyAssignFromSameType(const Value& that) {
}
}
-void Value::InternalMoveAssignFromSameType(Value&& that) {
- CHECK_EQ(type_, that.type_);
-
- switch (type_) {
- case Type::NONE:
- case Type::BOOLEAN:
- case Type::INTEGER:
- case Type::DOUBLE:
- InternalCopyFundamentalValue(that);
- return;
-
- case Type::STRING:
- *string_value_ = std::move(*that.string_value_);
- return;
- case Type::BINARY:
- *binary_value_ = std::move(*that.binary_value_);
- return;
- case Type::DICTIONARY:
- *dict_ptr_ = std::move(*that.dict_ptr_);
- return;
- case Type::LIST:
- *list_ = std::move(*that.list_);
- return;
- }
-}
-
void Value::InternalCleanup() {
switch (type_) {
case Type::NONE:
« no previous file with comments | « base/values.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698