Inline FundamentalValue into base::Value

skia/ext/benchmarking_canvas.cc

 // Copyright (c) 2013 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 "skia/ext/benchmarking_canvas.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/logging.h"
@@ skipping 31 matching lines @@
   }
 
 private:
   char separator_;
   std::ostringstream oss_;
 };
 
 std::unique_ptr<base::Value> AsValue(bool b) {
   std::unique_ptr<base::FundamentalValue> val(new base::FundamentalValue(b));
 
-  return std::move(val);
+  return val;
 }
 
 std::unique_ptr<base::Value> AsValue(SkScalar scalar) {
   std::unique_ptr<base::FundamentalValue> val(
       new base::FundamentalValue(scalar));
 
-  return std::move(val);

[jdoerrie, 2017/01/21 15:06:51]

After the typdef this results in a compile error: moving a local object in a
return statement prevents copy elision [-Werror,-Wpessimizing-move]

Therefore this change is necessary.

[brettw, 2017/01/23 05:52:50]

Interesting, I wonder why it didn't complain before, it should definitely be
changed.

[jdoerrie, 2017/01/24 08:59:35]

If I understand this correctly, only after the typedef the type of |val| is
equal to the function return type, resulting in a possibility for return value
optimization (RVO). The presence of |std::move| would prevent RVO, which is why
clang issues a warning.

For the other derived classes (such as |DictionaryValue|) no typedef to |Value|
exists, resulting in the type of |val| to be different than the function return
type. Hence |std::move| does not block RVO, because it could not happen in the
first place.

Reference:
- http://stackoverflow.com/a/12953145

+  return val;
 }
 
 std::unique_ptr<base::Value> AsValue(const SkSize& size) {
   std::unique_ptr<base::DictionaryValue> val(new base::DictionaryValue());
   val->Set("width",  AsValue(size.width()));
   val->Set("height", AsValue(size.height()));
 
   return std::move(val);
 }
 
@@ skipping 646 matching lines @@
   DCHECK(blob);
   AutoOp op(this, "DrawTextBlob", &paint);
   op.addParam("blob", AsValue(*blob));
   op.addParam("x", AsValue(x));
   op.addParam("y", AsValue(y));
 
   INHERITED::onDrawTextBlob(blob, x, y, *op.paint());
 }
 
 } // namespace skia