[core] Make enum ↔ string conversion more generic-friendly

Rewrite enum.hpp in such a way that parseConstant can be defined generically for all enumerated types.

While there, properly validated enumerated property values.

include/mbgl/platform/event.hpp

@@ -1,7 +1,5 @@
 #pragma once
 
-#include <mbgl/util/enum.hpp>
-
 #include <cstdint>
 
 namespace mbgl {
@@ -13,14 +11,6 @@ enum class EventSeverity : uint8_t {
     Error,
 };
 
-MBGL_DEFINE_ENUM_CLASS(EventSeverityClass, EventSeverity, {
-    { EventSeverity::Debug, "DEBUG" },
-    { EventSeverity::Info, "INFO" },
-    { EventSeverity::Warning, "WARNING" },
-    { EventSeverity::Error, "ERROR" },
-    { EventSeverity(-1), "UNKNOWN" },
-});
-
 enum class Event : uint8_t {
     General,
     Setup,
@@ -40,27 +30,6 @@ enum class Event : uint8_t {
     Glyph,
 };
 
-MBGL_DEFINE_ENUM_CLASS(EventClass, Event, {
-    { Event::General, "General" },
-    { Event::Setup, "Setup" },
-    { Event::Shader, "Shader" },
-    { Event::ParseStyle, "ParseStyle" },
-    { Event::ParseTile, "ParseTile" },
-    { Event::Render, "Render" },
-    { Event::Style, "Style" },
-    { Event::Database, "Database" },
-    { Event::HttpRequest, "HttpRequest" },
-    { Event::Sprite, "Sprite" },
-    { Event::Image, "Image" },
-    { Event::OpenGL, "OpenGL" },
-    { Event::JNI, "JNI" },
-    { Event::Android, "Android" },
-    { Event::Crash, "Crash" },
-    { Event::Glyph, "Glyph" },
-    { Event(-1), "Unknown" },
-});
-
-
 struct EventPermutation {
     const EventSeverity severity;
     const Event event;

include/mbgl/style/types.hpp

@@ -1,6 +1,6 @@
 #pragma once
 
-#include <mbgl/util/enum.hpp>
+#include <cstdint>
 
 namespace mbgl {
 
@@ -13,14 +13,6 @@ enum class SourceType : uint8_t {
     Annotations
 };
 
-MBGL_DEFINE_ENUM_CLASS(SourceTypeClass, SourceType, {
-    { SourceType::Vector, "vector" },
-    { SourceType::Raster, "raster" },
-    { SourceType::GeoJSON, "geojson" },
-    { SourceType::Video, "video" },
-    { SourceType::Annotations, "annotations" },
-});
-
 namespace style {
 
 enum class VisibilityType : bool {

include/mbgl/util/enum.hpp

@@ -1,52 +1,36 @@
 #pragma once
 
-#include <iosfwd>
+#include <mbgl/util/optional.hpp>
+
+#include <algorithm>
+#include <cassert>
 #include <string>
 
 namespace mbgl {
 
-template <typename Type>
-struct EnumValue {
-    const Type value;
-    const char *name;
-};
-
-template <typename EnumName, const EnumValue<EnumName> *names, const size_t length>
-struct Enum {
-    using Type = EnumName;
-    Type value;
-    static const constexpr size_t l = length;
-private:
-    static constexpr inline bool compare(const char *a, const char *b) {
-        return *a == *b && (*a == '\0' || compare(a + 1, b + 1));
-    }
-    static constexpr inline const char *lookup_type(Type e, EnumValue<Type> const * const list, size_t r) {
-        return r == 0 ? "" : list->value == e ? list->name : lookup_type(e, list + 1, r - 1);
-    }
-    static constexpr inline Type lookup_name(const char *n, EnumValue<Type> const * const list, size_t r) {
-        return r == 0 ? Type(-1) : compare(list->name, n) ? list->value : lookup_name(n, list + 1, r - 1);
-    }
+template <typename T>
+class Enum {
 public:
-    inline constexpr Enum(const char *n) : value(lookup_name(n, names, length)) {}
-    inline constexpr Enum(const std::string &n) : value(lookup_name(n.c_str(), names, length)) {}
-    inline constexpr Enum(Type t) : value(t) {}
-
-    inline void operator=(const char *n) { value = lookup_name(n, names, length); }
-    inline void operator=(const std::string &n) { *this = n.c_str(); }
-    inline void operator=(Type t) { value = t; }
+    using Value = std::pair<const T, const char *>;
 
-    inline constexpr bool valid() const { return value != Type(-1); }
+    static const char * toString(T t) {
+        auto it = std::find_if(begin, end, [&] (const auto& v) { return t == v.first; });
+        assert(it != end); return it->second;
+    }
 
-    inline constexpr const char *c_str() const { return lookup_type(value, names, length); }
-    inline std::string str() const { return c_str(); }
+    static optional<T> toEnum(const std::string& s) {
+        auto it = std::find_if(begin, end, [&] (const auto& v) { return s == v.second; });
+        return it == end ? optional<T>() : it->first;
+    }
 
-    inline constexpr operator Type() const { return value; }
+private:
+    static const Value* begin;
+    static const Value* end;
 };
 
-#define MBGL_DEFINE_ENUM_CLASS(name, type, strings...) \
-    const constexpr ::mbgl::EnumValue<type> type##_names[] = strings; \
-    using name = ::mbgl::Enum<type, type##_names, sizeof(type##_names) / sizeof(::mbgl::EnumValue<type>)>; \
-    inline std::ostream& operator<<(std::ostream& os, type t) { return os << name(t).str(); }
+#define MBGL_DEFINE_ENUM(type, strings...) \
+const constexpr Enum<type>::Value type##_names[] = strings; \
+template <> const Enum<type>::Value* Enum<type>::begin = std::begin(type##_names); \
+template <> const Enum<type>::Value* Enum<type>::end = std::end(type##_names);
 
 } // namespace mbgl
-

platform/darwin/src/log_nslog.mm

@@ -1,4 +1,5 @@
 #include <mbgl/platform/log.hpp>
+#include <mbgl/util/enum.hpp>
 
 #import <Foundation/Foundation.h>
 
@@ -7,7 +8,7 @@
 void Log::platformRecord(EventSeverity severity, const std::string &msg) {
     NSString *message =
         [[NSString alloc] initWithBytes:msg.data() length:msg.size() encoding:NSUTF8StringEncoding];
-    NSLog(@"[%s] %@", EventSeverityClass(severity).c_str(), message);
+    NSLog(@"[%s] %@", Enum<EventSeverity>::toString(severity), message);
 }
 
 }

platform/default/log_stderr.cpp

@@ -1,11 +1,12 @@
 #include <mbgl/platform/log.hpp>
+#include <mbgl/util/enum.hpp>
 
 #include <iostream>
 
 namespace mbgl {
 
 void Log::platformRecord(EventSeverity severity, const std::string &msg) {
-    std::cerr << "[" << severity << "] " << msg << std::endl;
+    std::cerr << "[" << Enum<EventSeverity>::toString(severity) << "] " << msg << std::endl;
 }
 
 } // namespace mbgl

platform/node/src/node_log.cpp

@@ -1,6 +1,8 @@
 #include "node_log.hpp"
 #include "util/async_queue.hpp"
 
+#include <mbgl/util/enum.hpp>
+
 namespace node_mbgl {
 
 struct NodeLogObserver::LogMessage {
@@ -23,10 +25,10 @@ NodeLogObserver::NodeLogObserver(v8::Local<v8::Object> target)
           auto msg = Nan::New<v8::Object>();
 
           Nan::Set(msg, Nan::New("class").ToLocalChecked(),
-              Nan::New(mbgl::EventClass(message.event).c_str()).ToLocalChecked());
+              Nan::New(mbgl::Enum<mbgl::Event>::toString(message.event)).ToLocalChecked());
 
           Nan::Set(msg, Nan::New("severity").ToLocalChecked(),
-              Nan::New(mbgl::EventSeverityClass(message.severity).c_str()).ToLocalChecked());
+              Nan::New(mbgl::Enum<mbgl::EventSeverity>::toString(message.severity)).ToLocalChecked());
 
           if (message.code != -1) {
               Nan::Set(msg, Nan::New("code").ToLocalChecked(),

src/mbgl/platform/event.cpp

@@ -0,0 +1,34 @@
+#include <mbgl/platform/event.hpp>
+#include <mbgl/util/enum.hpp>
+
+namespace mbgl {
+
+MBGL_DEFINE_ENUM(EventSeverity, {
+    { EventSeverity::Debug, "DEBUG" },
+    { EventSeverity::Info, "INFO" },
+    { EventSeverity::Warning, "WARNING" },
+    { EventSeverity::Error, "ERROR" },
+    { EventSeverity(-1), "UNKNOWN" },
+});
+
+MBGL_DEFINE_ENUM(Event, {
+    { Event::General, "General" },
+    { Event::Setup, "Setup" },
+    { Event::Shader, "Shader" },
+    { Event::ParseStyle, "ParseStyle" },
+    { Event::ParseTile, "ParseTile" },
+    { Event::Render, "Render" },
+    { Event::Style, "Style" },
+    { Event::Database, "Database" },
+    { Event::HttpRequest, "HttpRequest" },
+    { Event::Sprite, "Sprite" },
+    { Event::Image, "Image" },
+    { Event::OpenGL, "OpenGL" },
+    { Event::JNI, "JNI" },
+    { Event::Android, "Android" },
+    { Event::Crash, "Crash" },
+    { Event::Glyph, "Glyph" },
+    { Event(-1), "Unknown" },
+});
+
+} // namespace mbgl

src/mbgl/platform/log.cpp

@@ -1,4 +1,5 @@
 #include <mbgl/platform/log.hpp>
+#include <mbgl/util/enum.hpp>
 
 #include <cstdio>
 #include <cstdarg>
@@ -54,7 +55,7 @@ void Log::record(EventSeverity severity, Event event, int64_t code, const std::s
     logStream << "{" << name << "}";
     #endif
 
-    logStream << "[" << event << "]";
+    logStream << "[" << Enum<Event>::toString(event) << "]";
 
     if (code >= 0) {
         logStream << "(" << code << ")";

src/mbgl/style/parser.cpp

@@ -14,6 +14,7 @@
 
 #include <mbgl/tile/geometry_tile.hpp>
 #include <mbgl/util/mapbox.hpp>
+#include <mbgl/util/enum.hpp>
 
 #include <rapidjson/document.h>
 #include <rapidjson/error/en.h>
@@ -161,11 +162,15 @@ void Parser::parseSources(const JSValue& value) {
 
         const JSValue& typeVal = sourceVal["type"];
         if (!typeVal.IsString()) {
-            Log::Warning(Event::ParseStyle, "source type must have one of the enum values");
+            Log::Warning(Event::ParseStyle, "source type must be a string");
             continue;
         }
 
-        const auto type = SourceTypeClass({ typeVal.GetString(), typeVal.GetStringLength() });
+        const auto type = Enum<SourceType>::toEnum({ typeVal.GetString(), typeVal.GetStringLength() });
+        if (!type) {
+            Log::Warning(Event::ParseStyle, "source type must have one of the enum values");
+            continue;
+        }
 
         // Sources can have URLs, either because they reference an external TileJSON file, or
         // because reference a GeoJSON file. They don't have to have one though when all source
@@ -177,7 +182,7 @@ void Parser::parseSources(const JSValue& value) {
         std::unique_ptr<Tileset> tileset;
         std::unique_ptr<mapbox::geojsonvt::GeoJSONVT> geojsonvt;
 
-        switch (type) {
+        switch (*type) {
         case SourceType::Raster:
             if (sourceVal.HasMember("tileSize")) {
                 const JSValue& tileSizeVal = sourceVal["tileSize"];
@@ -236,7 +241,7 @@ void Parser::parseSources(const JSValue& value) {
         }
 
         const std::string id { nameVal.GetString(), nameVal.GetStringLength() };
-        std::unique_ptr<Source> source = std::make_unique<Source>(type, id, url, tileSize, std::move(tileset), std::move(geojsonvt));
+        std::unique_ptr<Source> source = std::make_unique<Source>(*type, id, url, tileSize, std::move(tileset), std::move(geojsonvt));
 
         sourcesMap.emplace(id, source.get());
         sources.emplace_back(std::move(source));
@@ -481,21 +486,25 @@ void Parser::parseLayer(const std::string& id, const JSValue& value, std::unique
     }
 }
 
-MBGL_DEFINE_ENUM_CLASS(VisibilityTypeClass, VisibilityType, {
-    { VisibilityType::Visible, "visible" },
-    { VisibilityType::None, "none" },
-});
-
 void Parser::parseVisibility(Layer& layer, const JSValue& value) {
     Layer::Impl& impl = *layer.baseImpl;
+
     if (!value.HasMember("visibility")) {
         return;
-    } else if (!value["visibility"].IsString()) {
+    }
+
+    if (!value["visibility"].IsString()) {
         Log::Warning(Event::ParseStyle, "value of 'visibility' must be a string");
-        impl.visibility = VisibilityType::Visible;
         return;
     }
-    impl.visibility = VisibilityTypeClass({ value["visibility"].GetString(), value["visibility"].GetStringLength() });
+
+    const auto enumValue = Enum<VisibilityType>::toEnum({ value["visibility"].GetString(), value["visibility"].GetStringLength() });
+    if (!enumValue) {
+        Log::Warning(Event::ParseStyle, "value of 'visibility' must be a valid enumeration value");
+        return;
+    }
+
+    impl.visibility = *enumValue;
 }
 
 Value parseFeatureType(const Value& value) {