3 Commits 42f5d822b2 ... 0717490adf

Author SHA1 Message Date
  Sam Jaffe 0717490adf refactor: cleanup and alignment guarantees 1 month ago
  Sam Jaffe 666cd18351 test-fix: properly match the output order due to sorting changes 1 month ago
  Sam Jaffe e3acbe59f8 refactor: replace PropertiesContraint and DependenciesConstraint members w/ unordered_map 1 month ago

+ 27 - 16
include/jvalidate/constraint.h

@@ -36,6 +36,7 @@
 
 #define DECLARE_TYPE_ENUM(X) X,
 #define SIZEOF_STRUCT(X) sizeof(X),
+#define ALIGNOF_STRUCT(X) alignof(X),
 #define SWITCH_DELETE(X)                                                                           \
   case Type::X:                                                                                    \
     reinterpret_cast<X const *>(data_.data())->~X();                                               \
@@ -51,6 +52,10 @@
 namespace jvalidate::constraint {
 
 class Constraint {
+public:
+  static constexpr size_t UNION_WIDTH = std::max({CONSTRAINT_IMPLEMENTATION_LIST(SIZEOF_STRUCT)});
+  static constexpr size_t UNION_ALIGN = std::max({CONSTRAINT_IMPLEMENTATION_LIST(ALIGNOF_STRUCT)});
+
 public:
   CONSTRAINT_IMPLEMENTATION_LIST(IMPLICIT_CONSTRUCTOR)
   Constraint(Constraint const &) = delete;
@@ -88,7 +93,7 @@ private:
   enum class Type : uint8_t { None, CONSTRAINT_IMPLEMENTATION_LIST(DECLARE_TYPE_ENUM) };
 
   Type type_ = Type::None;
-  std::array<std::byte, std::max({CONSTRAINT_IMPLEMENTATION_LIST(SIZEOF_STRUCT)})> data_;
+  alignas(UNION_ALIGN) std::array<std::byte, UNION_WIDTH> data_;
 };
 }
 
@@ -1154,12 +1159,12 @@ public:
   static auto properties(detail::ParserContext<A> const & context) {
     EXPECT(context.schema.type() == adapter::Type::Object);
 
-    std::map<std::string, schema::Node const *> rval;
+    constraint::PropertiesConstraint rval;
     for (auto [prop, subschema] : context.schema.as_object()) {
-      rval.emplace(prop, context.child(subschema, prop).node());
+      rval.properties.emplace(prop, context.child(subschema, prop).node());
     }
 
-    return ptr(constraint::PropertiesConstraint{rval});
+    return ptr(std::move(rval));
   }
 
   /**
@@ -1310,26 +1315,25 @@ public:
   static auto dependencies(detail::ParserContext<A> const & context) {
     EXPECT(context.schema.type() == adapter::Type::Object);
 
-    std::map<std::string, schema::Node const *> schemas;
-    std::map<std::string, std::unordered_set<std::string>> required;
+    constraint::DependenciesConstraint rval;
     for (auto [prop, subschema] : context.schema.as_object()) {
       if (subschema.type() == adapter::Type::Array) {
         // Option 1) dependentRequired
         for (auto key : subschema.as_array()) {
           EXPECT(key.type() == adapter::Type::String);
-          required[prop].insert(key.as_string());
+          rval.required[prop].insert(key.as_string());
         }
       } else if (context.vocab->version() <= schema::Version::Draft03 &&
                  subschema.type() == adapter::Type::String) {
         // Option 2) Special single-element dependentRequired in Draft03
-        required[prop].insert(subschema.as_string());
+        rval.required[prop].insert(subschema.as_string());
       } else {
         // Option 3) dependentSchemas
-        schemas.emplace(prop, context.child(subschema, prop).node());
+        rval.subschemas.emplace(prop, context.child(subschema, prop).node());
       }
     }
 
-    return ptr(constraint::DependenciesConstraint{.subschemas = schemas, .required = required});
+    return ptr(std::move(rval));
   }
 
   /**
@@ -1355,12 +1359,12 @@ public:
   static auto dependentSchemas(detail::ParserContext<A> const & context) {
     EXPECT(context.schema.type() == adapter::Type::Object);
 
-    std::map<std::string, schema::Node const *> rval;
+    constraint::DependenciesConstraint rval;
     for (auto [prop, subschema] : context.schema.as_object()) {
-      rval.emplace(prop, context.child(subschema, prop).node());
+      rval.subschemas.emplace(prop, context.child(subschema, prop).node());
     }
 
-    return ptr(constraint::DependenciesConstraint{.subschemas = rval});
+    return ptr(std::move(rval));
   }
 
   /**
@@ -1385,17 +1389,24 @@ public:
   static auto dependentRequired(detail::ParserContext<A> const & context) {
     EXPECT(context.schema.type() == adapter::Type::Object);
 
-    std::map<std::string, std::unordered_set<std::string>> rval;
+    constraint::DependenciesConstraint rval;
     for (auto [prop, subschema] : context.schema.as_object()) {
       EXPECT(subschema.type() == adapter::Type::Array);
       for (auto key : subschema.as_array()) {
         EXPECT(key.type() == adapter::Type::String);
-        rval[prop].insert(key.as_string());
+        rval.required[prop].insert(key.as_string());
       }
     }
 
-    return ptr(constraint::DependenciesConstraint{.subschemas = {}, .required = rval});
+    return ptr(std::move(rval));
   }
 };
 }
+
+#undef DECLARE_TYPE_ENUM
+#undef SIZEOF_STRUCT
+#undef ALIGNOF_STRUCT
+#undef SWITCH_DELETE
+#undef SWITCH_VISIT
+#undef IMPLICIT_CONSTRUCTOR
 // NOLINTEND(readability-identifier-naming)

+ 3 - 3
include/jvalidate/constraint/object_constraint.h

@@ -38,8 +38,8 @@ struct AdditionalPropertiesConstraint {
  * https://json-schema.org/draft/2020-12/json-schema-validation#section-6.5.4
  */
 struct DependenciesConstraint {
-  std::map<std::string, schema::Node const *> subschemas;
-  std::map<std::string, std::unordered_set<std::string>> required;
+  std::unordered_map<std::string, schema::Node const *> subschemas;
+  std::unordered_map<std::string, std::unordered_set<std::string>> required;
 };
 
 /**
@@ -87,7 +87,7 @@ struct PatternPropertiesConstraint {
  * https://json-schema.org/draft/2020-12/json-schema-core#section-10.3.2.1
  */
 struct PropertiesConstraint {
-  std::map<std::string, schema::Node const *> properties;
+  std::unordered_map<std::string, schema::Node const *> properties;
 };
 
 /**

+ 0 - 1
include/jvalidate/detail/dynamic_reference_context.h

@@ -2,7 +2,6 @@
 
 #include <cstdlib>
 #include <deque>
-#include <map>
 #include <optional>
 
 #include <jvalidate/detail/anchor.h>

+ 0 - 10
include/jvalidate/detail/iostream.h

@@ -52,16 +52,6 @@ inline std::ostream & operator<<(std::ostream & os, Version version) {
 }
 
 namespace jvalidate {
-inline std::ostream & operator<<(std::ostream & os, Status stat) {
-  if (stat == Status::Accept) {
-    return os << "Accept";
-  }
-  if (stat == Status::Reject) {
-    return os << "Reject";
-  }
-  return os << "Noop";
-}
-
 template <typename T>
 inline std::ostream & operator<<(std::ostream & os, std::set<T> const & data) {
   if (data.size() == 1) {

+ 5 - 8
include/jvalidate/validation_result.h

@@ -2,7 +2,6 @@
 
 #include <algorithm>
 #include <cstdlib>
-#include <map>
 #include <ostream>
 #include <string>
 #include <unordered_map>
@@ -87,14 +86,11 @@ public:
    * }
    */
   friend std::ostream & operator<<(std::ostream & os, ValidationResult const & result) {
-    auto sorted = []<typename K, typename V>(std::unordered_map<K, V> const & container) {
-      return std::map<K, V const &>(container.begin(), container.end());
-    };
     char const * div = "\n";
     os << "{\n" << indent(1) << R"("valid": )" << (result.valid_ ? "true" : "false") << ',' << '\n';
     os << indent(1) << R"("details": [)";
-    for (auto const & [doc_path, by_schema] : sorted(result.results_)) {
-      for (auto const & [schema_path, local] : sorted(by_schema)) {
+    for (auto const & [doc_path, by_schema] : result.results_) {
+      for (auto const & [schema_path, local] : by_schema) {
         os << std::exchange(div, ",\n") << indent(2) << '{' << '\n';
         os << indent(3) << R"("valid": )" << (local.valid ? "true" : "false") << ',' << '\n';
         os << indent(3) << R"("evaluationPath": ")" << schema_path << '"' << ',' << '\n';
@@ -227,8 +223,9 @@ public:
 
 private:
   /**
-   * @brief Transfer the contents of another ValidationResult into this one using
-   * {@see std::map::merge} to transfer the data minimizing the need for copy/move.
+   * @brief Transfer the contents of another ValidationResult into this one
+   * using {@see std::unordered_map::merge} to transfer the data minimizing the
+   * need for copy/move.
    *
    * @param result The ValidationResult being consumed
    */

+ 12 - 17
include/jvalidate/validation_visitor.h

@@ -485,28 +485,23 @@ private:
                Adapter auto const & document) const {
     NOOP_UNLESS_TYPE(Object);
 
-    auto object = document.as_object();
     Status rval = Status::Accept;
-    for (auto const & [key, subschema] : cons.subschemas) {
-      if (not object.contains(key)) {
-        continue;
-      }
-
-      rval &= validate_subschema(subschema, document, key);
-      BREAK_EARLY_IF_NO_RESULT_TREE();
-    }
 
-    for (auto [key, required] : cons.required) {
-      if (not object.contains(key)) {
-        continue;
+    std::unordered_set<std::string> properties;
+    for (auto const & [key, child] : document.as_object()) {
+      properties.insert(key);
+      if (auto it = cons.subschemas.find(key); it != cons.subschemas.end()) {
+        rval &= validate_subschema(it->second, document, key);
+        BREAK_EARLY_IF_NO_RESULT_TREE();
       }
+    }
 
-      for (auto const & [key, _] : object) {
-        required.erase(key);
+    auto has_property = [&properties](auto const & key) { return properties.contains(key); };
+    for (auto const & [key, required] : cons.required) {
+      if (has_property(key) and not std::ranges::all_of(required, has_property)) {
+        rval &= required.empty();
+        BREAK_EARLY_IF_NO_RESULT_TREE();
       }
-
-      rval &= required.empty();
-      BREAK_EARLY_IF_NO_RESULT_TREE();
     }
 
     return rval;

+ 2 - 2
tests/annotation_test.cxx

@@ -318,7 +318,7 @@ TEST(ValidationResultJsonTest, OutputsFormattedJSON) {
     {
       "valid": false,
       "evaluationPath": "/items",
-      "instanceLocation": "/0",
+      "instanceLocation": "/1",
       "errors": {
         "": "always false"
       }
@@ -326,7 +326,7 @@ TEST(ValidationResultJsonTest, OutputsFormattedJSON) {
     {
       "valid": false,
       "evaluationPath": "/items",
-      "instanceLocation": "/1",
+      "instanceLocation": "/0",
       "errors": {
         "": "always false"
       }