Inline FundamentalValue into base::Value

base/values.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/values.h"
 
 #include <string.h>
 
 #include <algorithm>
 #include <cmath>
@@ skipping 57 matching lines @@
       return CopyDictionaryWithoutEmptyChildren(
           static_cast<const DictionaryValue&>(node));
 
     default:
       return node.CreateDeepCopy();
   }
 }
 
 }  // namespace
 
+Value::Value(bool in_bool) : type_(Type::BOOLEAN), bool_value_(in_bool) {}
+
+Value::Value(int in_int) : type_(Type::INTEGER), int_value_(in_int) {}
+
+Value::Value(double in_double) : type_(Type::DOUBLE), double_value_(in_double) {
+  if (!std::isfinite(double_value_)) {
+    NOTREACHED() << "Non-finite (i.e. NaN or positive/negative infinity) "
+                 << "values cannot be represented in JSON";
+    double_value_ = 0.0;
+  }
+}
+
 Value::~Value() {
 }
 
+void Value::InternalCopyFrom(const Value& that) {
+  type_ = that.type_;
+  switch (type_) {
+    case Type::NONE:
+      // Nothing to do
+      return;
+
+    case Type::BOOLEAN:
+      bool_value_ = that.bool_value_;
+      return;
+    case Type::INTEGER:
+      int_value_ = that.int_value_;
+      return;
+    case Type::DOUBLE:
+      double_value_ = that.double_value_;
+      return;
+
+    // TODO(crbug.com/646113): Implement these once the corresponding derived
+    // classes are removed.
+    case Type::STRING:
+    case Type::BINARY:
+    case Type::LIST:
+    case Type::DICTIONARY:
+      return;
+  }
+}
+
 // static
 std::unique_ptr<Value> Value::CreateNullValue() {
   return WrapUnique(new Value(Type::NONE));
 }
 
 // static
 const char* Value::GetTypeName(Value::Type type) {
   DCHECK_GE(static_cast<int>(type), 0);
   DCHECK_LT(static_cast<size_t>(type), arraysize(kTypeNames));
   return kTypeNames[static_cast<size_t>(type)];
 }
 
+bool Value::GetBool() const {
+  CHECK(is_bool());
+  return bool_value_;
+}
+
+int Value::GetInt() const {
+  CHECK(is_int());
+  return int_value_;
+}
+
+double Value::GetDouble() const {
+  if (is_double())
+    return double_value_;
+  if (is_int())
+    return int_value_;
+  CHECK(false);
+  return 0.0;
+}
+
 bool Value::GetAsBinary(const BinaryValue** out_value) const {
   return false;
 }
 
 bool Value::GetAsBoolean(bool* out_value) const {
-  return false;
+  if (out_value && is_bool()) {
+    *out_value = bool_value_;
+    return true;
+  }
+  return is_bool();
 }
 
 bool Value::GetAsInteger(int* out_value) const {
-  return false;
+  if (out_value && is_int()) {
+    *out_value = int_value_;
+    return true;
+  }
+  return is_int();
 }
 
 bool Value::GetAsDouble(double* out_value) const {
-  return false;
+  if (out_value && is_double()) {
+    *out_value = double_value_;
+    return true;
+  } else if (out_value && is_int()) {
+    // Allow promotion from int to double.
+    *out_value = int_value_;
+    return true;
+  }
+  return is_double() || is_int();
 }
 
 bool Value::GetAsString(std::string* out_value) const {
   return false;
 }
 
 bool Value::GetAsString(string16* out_value) const {
   return false;
 }
 
@@ skipping 13 matching lines @@
   return false;
 }
 
 bool Value::GetAsDictionary(const DictionaryValue** out_value) const {
   return false;
 }
 
 Value* Value::DeepCopy() const {
   // This method should only be getting called for null Values--all subclasses
   // need to provide their own implementation;.
-  DCHECK(IsType(Type::NONE));
-  return CreateNullValue().release();
+  switch (type()) {
+    case Type::NONE:
+      return CreateNullValue().release();
+
+    // For now, make FundamentalValues for backward-compatibility. Convert to
+    // Value when that code is deleted.
+    case Type::BOOLEAN:
+      return new FundamentalValue(bool_value_);
+    case Type::INTEGER:
+      return new FundamentalValue(int_value_);
+    case Type::DOUBLE:
+      return new FundamentalValue(double_value_);
+
+    default:
+      // All other types should be handled by subclasses.
+      NOTREACHED();
+      return nullptr;
+  }
 }
 
 std::unique_ptr<Value> Value::CreateDeepCopy() const {
   return WrapUnique(DeepCopy());
 }
 
 bool Value::Equals(const Value* other) const {
-  // This method should only be getting called for null Values--all subclasses
-  // need to provide their own implementation;.
-  DCHECK(IsType(Type::NONE));
-  return other->IsType(Type::NONE);
+  if (other->type() != type())
+    return false;
+
+  switch (type()) {
+    case Type::NONE:
+      return true;
+    case Type::BOOLEAN:
+      return bool_value_ == other->bool_value_;
+    case Type::INTEGER:
+      return int_value_ == other->int_value_;
+    case Type::DOUBLE:
+      return double_value_ == other->double_value_;
+    default:
+      // This method should only be getting called for the above types -- all
+      // subclasses need to provide their own implementation;.
+      NOTREACHED();
+      return false;
+  }
 }
 
 // static
 bool Value::Equals(const Value* a, const Value* b) {
   if ((a == NULL) && (b == NULL)) return true;
   if ((a == NULL) ^  (b == NULL)) return false;
   return a->Equals(b);
 }
 
 Value::Value(Type type) : type_(type) {}

[jdoerrie, 2016/12/23 14:18:26]

According to the proposal this constructor should default initialize the
corresponding data member:

"""
explicit Value(Type type);  // Does default value for this type.
"""

Right now it is protected, so customers would not be able to use it, but maybe
it's worth implementing regardless? There is a chance a lot of currently
protected constructors will become public when the inline StringValue CL lands. 

[vabr (Chromium), 2016/12/23 21:34:22]

Seems fine with me, done. brettw@ -- please let me know if you disagree (or have
comments to the particular way I wrote this).

 
-Value::Value(const Value& that) : type_(that.type_) {}
+Value::Value(const Value& that) {
+  InternalCopyFrom(that);
+}
+
+Value::Value(Value&& that) {
+  // TODO(crbug.com/646113): Implement InternalMoveFrom for types where moving
+  // and copying differ.
+  InternalCopyFrom(that);
+}
 
 Value& Value::operator=(const Value& that) {
-  type_ = that.type_;
+  if (this != &that)
+    InternalCopyFrom(that);
+
+  return *this;
+}
+
+Value& Value::operator=(Value&& that) {
+  if (this != &that) {
+    // TODO(crbug.com/646113): Implement InternalMoveFrom for types where moving
+    // and copying differ.
+    InternalCopyFrom(that);
+  }
+
   return *this;
 }
 
 ///////////////////// FundamentalValue ////////////////////
 
-FundamentalValue::FundamentalValue(bool in_value)
-    : Value(Type::BOOLEAN), boolean_value_(in_value) {}
+FundamentalValue::FundamentalValue(bool in_value) : Value(in_value) {}
 
-FundamentalValue::FundamentalValue(int in_value)
-    : Value(Type::INTEGER), integer_value_(in_value) {}
+FundamentalValue::FundamentalValue(int in_value) : Value(in_value) {}
 
-FundamentalValue::FundamentalValue(double in_value)
-    : Value(Type::DOUBLE), double_value_(in_value) {
-  if (!std::isfinite(double_value_)) {
-    NOTREACHED() << "Non-finite (i.e. NaN or positive/negative infinity) "
-                 << "values cannot be represented in JSON";
-    double_value_ = 0.0;
-  }
-}
+FundamentalValue::FundamentalValue(double in_value) : Value(in_value) {}
 
-FundamentalValue::~FundamentalValue() {
-}
-
-bool FundamentalValue::GetAsBoolean(bool* out_value) const {
-  if (out_value && IsType(Type::BOOLEAN))
-    *out_value = boolean_value_;
-  return (IsType(Type::BOOLEAN));
-}
-
-bool FundamentalValue::GetAsInteger(int* out_value) const {
-  if (out_value && IsType(Type::INTEGER))
-    *out_value = integer_value_;
-  return (IsType(Type::INTEGER));
-}
-
-bool FundamentalValue::GetAsDouble(double* out_value) const {
-  if (out_value && IsType(Type::DOUBLE))
-    *out_value = double_value_;
-  else if (out_value && IsType(Type::INTEGER))
-    *out_value = integer_value_;
-  return (IsType(Type::DOUBLE) || IsType(Type::INTEGER));
-}
-
-FundamentalValue* FundamentalValue::DeepCopy() const {
-  switch (GetType()) {
-    case Type::BOOLEAN:
-      return new FundamentalValue(boolean_value_);
-
-    case Type::INTEGER:
-      return new FundamentalValue(integer_value_);
-
-    case Type::DOUBLE:
-      return new FundamentalValue(double_value_);
-
-    default:
-      NOTREACHED();
-      return NULL;
-  }
-}
-
-bool FundamentalValue::Equals(const Value* other) const {
-  if (other->GetType() != GetType())
-    return false;
-
-  switch (GetType()) {
-    case Type::BOOLEAN: {
-      bool lhs, rhs;
-      return GetAsBoolean(&lhs) && other->GetAsBoolean(&rhs) && lhs == rhs;
-    }
-    case Type::INTEGER: {
-      int lhs, rhs;
-      return GetAsInteger(&lhs) && other->GetAsInteger(&rhs) && lhs == rhs;
-    }
-    case Type::DOUBLE: {
-      double lhs, rhs;
-      return GetAsDouble(&lhs) && other->GetAsDouble(&rhs) && lhs == rhs;
-    }
-    default:
-      NOTREACHED();
-      return false;
-  }
-}
+FundamentalValue::~FundamentalValue() {}
 
 ///////////////////// StringValue ////////////////////
 
 StringValue::StringValue(StringPiece in_value)
     : Value(Type::STRING), value_(in_value.as_string()) {
   DCHECK(IsStringUTF8(in_value));
 }
 
 StringValue::StringValue(const string16& in_value)
     : Value(Type::STRING), value_(UTF16ToUTF8(in_value)) {}
@@ skipping 905 matching lines @@
 }
 
 std::ostream& operator<<(std::ostream& out, const Value::Type& type) {
   if (static_cast<int>(type) < 0 ||
       static_cast<size_t>(type) >= arraysize(kTypeNames))
     return out << "Invalid Type (index = " << static_cast<int>(type) << ")";
   return out << Value::GetTypeName(type);
 }
 
 }  // namespace base