From f210bbbb6c9532c5bdcf176e191bef9a0a9858c7 Mon Sep 17 00:00:00 2001
From: Ander Conselvan de Oliveira <ander.deoliveira@mapbox.com>
Date: Wed, 8 May 2019 14:00:48 +0300
Subject: [PATCH] [core] Don't use optional unique_ptr in function coversion
 code

Since unique_ptr is already nullable there is no need to wrap it in an
optional<>.
---
 include/mbgl/style/expression/dsl.hpp  | 14 ++++++-------
 src/mbgl/style/conversion/function.cpp | 27 ++++++++++++++------------
 src/mbgl/style/expression/dsl.cpp      | 20 +++++++++----------
 3 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/include/mbgl/style/expression/dsl.hpp b/include/mbgl/style/expression/dsl.hpp
index dc0f1ab90..bcab999ab 100644
--- a/include/mbgl/style/expression/dsl.hpp
+++ b/include/mbgl/style/expression/dsl.hpp
@@ -32,20 +32,20 @@ std::unique_ptr<Expression> literal(std::initializer_list<double> value);
 std::unique_ptr<Expression> literal(std::initializer_list<const char *> value);
 
 std::unique_ptr<Expression> assertion(type::Type, std::unique_ptr<Expression>,
-                                      optional<std::unique_ptr<Expression>> def = {});
+                                      std::unique_ptr<Expression> def = nullptr);
 std::unique_ptr<Expression> number(std::unique_ptr<Expression>,
-                                   optional<std::unique_ptr<Expression>> def = {});
+                                   std::unique_ptr<Expression> def = nullptr);
 std::unique_ptr<Expression> string(std::unique_ptr<Expression>,
-                                   optional<std::unique_ptr<Expression>> def = {});
+                                   std::unique_ptr<Expression> def = nullptr);
 std::unique_ptr<Expression> boolean(std::unique_ptr<Expression>,
-                                    optional<std::unique_ptr<Expression>> def = {});
+                                    std::unique_ptr<Expression> def = nullptr);
 
 std::unique_ptr<Expression> toColor(std::unique_ptr<Expression>,
-                                    optional<std::unique_ptr<Expression>> def = {});
+                                    std::unique_ptr<Expression> def = nullptr);
 std::unique_ptr<Expression> toString(std::unique_ptr<Expression>,
-                                     optional<std::unique_ptr<Expression>> def = {});
+                                     std::unique_ptr<Expression> def = nullptr);
 std::unique_ptr<Expression> toFormatted(std::unique_ptr<Expression>,
-                                        optional<std::unique_ptr<Expression>> def = {});
+                                        std::unique_ptr<Expression> def = nullptr);
     
 std::unique_ptr<Expression> get(const char* value);
 std::unique_ptr<Expression> get(std::unique_ptr<Expression>);
diff --git a/src/mbgl/style/conversion/function.cpp b/src/mbgl/style/conversion/function.cpp
index e6d23268b..086bc4e27 100644
--- a/src/mbgl/style/conversion/function.cpp
+++ b/src/mbgl/style/conversion/function.cpp
@@ -452,7 +452,7 @@ template <class T>
 std::unique_ptr<Expression> categorical(type::Type type,
                                         const std::string& property,
                                         std::map<T, std::unique_ptr<Expression>> branches,
-                                        optional<std::unique_ptr<Expression>> def) {
+                                        std::unique_ptr<Expression> def) {
     std::unordered_map<T, std::shared_ptr<Expression>> convertedBranches;
     for (auto& b : branches) {
         convertedBranches[b.first] = std::move(b.second);
@@ -460,14 +460,14 @@ std::unique_ptr<Expression> categorical(type::Type type,
     return std::make_unique<Match<T>>(type,
                                       get(literal(property)),
                                       std::move(convertedBranches),
-                                      def ? std::move(*def) : error("replaced with default"));
+                                      def ? std::move(def) : error("replaced with default"));
 }
 
 template <>
 std::unique_ptr<Expression> categorical<bool>(type::Type type,
                                               const std::string& property,
                                               std::map<bool, std::unique_ptr<Expression>> branches,
-                                              optional<std::unique_ptr<Expression>> def) {
+                                              std::unique_ptr<Expression> def) {
     auto it = branches.find(true);
     std::unique_ptr<Expression> trueCase = it == branches.end() ?
         error("replaced with default") :
@@ -483,13 +483,13 @@ std::unique_ptr<Expression> categorical<bool>(type::Type type,
     convertedBranches.emplace_back(eq(get(literal(property)), literal(Value(false))), std::move(falseCase));
 
     return std::make_unique<Case>(type, std::move(convertedBranches),
-                                  def ? std::move(*def) : error("replaced with default"));
+                                  def ? std::move(def) : error("replaced with default"));
 }
 
 static std::unique_ptr<Expression> numberOrDefault(type::Type type,
                                                    std::unique_ptr<Expression> get,
                                                    std::unique_ptr<Expression> expr,
-                                                   optional<std::unique_ptr<Expression>> def) {
+                                                   std::unique_ptr<Expression> def) {
     if (!def) {
         return expr;
     }
@@ -497,14 +497,14 @@ static std::unique_ptr<Expression> numberOrDefault(type::Type type,
     std::vector<Case::Branch> branches;
     branches.emplace_back(eq(compound("typeof", std::move(get)), literal("number")),
                           std::move(expr));
-    return std::make_unique<Case>(type, std::move(branches), std::move(*def));
+    return std::make_unique<Case>(type, std::move(branches), std::move(def));
 }
 
 static optional<std::unique_ptr<Expression>> convertIntervalFunction(type::Type type,
                                                                      const Convertible& value,
                                                                      Error& error,
                                                                      std::function<std::unique_ptr<Expression> (bool)> makeInput,
-                                                                     optional<std::unique_ptr<Expression>> def,
+                                                                     std::unique_ptr<Expression> def,
                                                                      bool convertTokens = false) {
     auto stops = convertStops(type, value, error, convertTokens);
     if (!stops) {
@@ -522,7 +522,7 @@ static optional<std::unique_ptr<Expression>> convertExponentialFunction(type::Ty
                                                                         const Convertible& value,
                                                                         Error& error,
                                                                         std::function<std::unique_ptr<Expression> (bool)> makeInput,
-                                                                        optional<std::unique_ptr<Expression>> def,
+                                                                        std::unique_ptr<Expression> def,
                                                                         bool convertTokens = false) {
     auto stops = convertStops(type, value, error, convertTokens);
     if (!stops) {
@@ -543,7 +543,7 @@ static optional<std::unique_ptr<Expression>> convertCategoricalFunction(type::Ty
                                                                         const Convertible& value,
                                                                         Error& err,
                                                                         const std::string& property,
-                                                                        optional<std::unique_ptr<Expression>> def) {
+                                                                        std::unique_ptr<Expression> def) {
     auto stopsValue = objectMember(value, "stops");
     if (!stopsValue) {
         err.message = "function value must specify stops";
@@ -709,12 +709,15 @@ optional<std::unique_ptr<Expression>> convertFunctionToExpression(type::Type typ
         }
     }
 
-    auto defaultExpr = [&]() -> optional<std::unique_ptr<Expression>> {
+    auto defaultExpr = [&]() -> std::unique_ptr<Expression> {
         auto member = objectMember(value, "default");
         if (member) {
-            return convertLiteral(type, *member, err);
+            auto literal = convertLiteral(type, *member, err);
+            if (literal) {
+                return std::move(*literal);
+            }
         }
-        return {};
+        return nullptr;
     };
 
     if (!objectMember(value, "property")) {
diff --git a/src/mbgl/style/expression/dsl.cpp b/src/mbgl/style/expression/dsl.cpp
index dd4aa7d27..70442de96 100644
--- a/src/mbgl/style/expression/dsl.cpp
+++ b/src/mbgl/style/expression/dsl.cpp
@@ -52,51 +52,51 @@ std::unique_ptr<Expression> literal(std::initializer_list<const char *> value) {
 
 std::unique_ptr<Expression> assertion(type::Type type,
                                       std::unique_ptr<Expression> value,
-                                      optional<std::unique_ptr<Expression>> def) {
+                                      std::unique_ptr<Expression> def) {
     std::vector<std::unique_ptr<Expression>> v  = vec(std::move(value));
     if (def) {
-        v.push_back(std::move(*def));
+        v.push_back(std::move(def));
     }
     return std::make_unique<Assertion>(type, std::move(v));
 }
 
 std::unique_ptr<Expression> number(std::unique_ptr<Expression> value,
-                                   optional<std::unique_ptr<Expression>> def) {
+                                   std::unique_ptr<Expression> def) {
     return assertion(type::Number, std::move(value), std::move(def));
 }
 
 std::unique_ptr<Expression> string(std::unique_ptr<Expression> value,
-                                   optional<std::unique_ptr<Expression>> def) {
+                                   std::unique_ptr<Expression> def) {
     return assertion(type::String, std::move(value), std::move(def));
 }
 
 std::unique_ptr<Expression> boolean(std::unique_ptr<Expression> value,
-                                    optional<std::unique_ptr<Expression>> def) {
+                                    std::unique_ptr<Expression> def) {
     return assertion(type::Boolean, std::move(value), std::move(def));
 }
 
 std::unique_ptr<Expression> coercion(type::Type type,
                                      std::unique_ptr<Expression> value,
-                                     optional<std::unique_ptr<Expression>> def) {
+                                     std::unique_ptr<Expression> def) {
     std::vector<std::unique_ptr<Expression>> v = vec(std::move(value));
     if (def) {
-        v.push_back(std::move(*def));
+        v.push_back(std::move(def));
     }
     return std::make_unique<Coercion>(type, std::move(v));
 }
 
 std::unique_ptr<Expression> toColor(std::unique_ptr<Expression> value,
-                                    optional<std::unique_ptr<Expression>> def) {
+                                    std::unique_ptr<Expression> def) {
     return coercion(type::Color, std::move(value), std::move(def));
 }
 
 std::unique_ptr<Expression> toString(std::unique_ptr<Expression> value,
-                                     optional<std::unique_ptr<Expression>> def) {
+                                     std::unique_ptr<Expression> def) {
     return coercion(type::String, std::move(value), std::move(def));
 }
 
 std::unique_ptr<Expression> toFormatted(std::unique_ptr<Expression> value,
-                                        optional<std::unique_ptr<Expression>> def) {
+                                        std::unique_ptr<Expression> def) {
     return coercion(type::Formatted, std::move(value), std::move(def));
 }