From 39d71b10f7e02955423d5eb2f6484f32ee5c6c90 Mon Sep 17 00:00:00 2001
From: Pranjal Raihan <praihan@meta.com>
Date: Wed, 15 Jan 2025 22:35:02 -0800
Subject: [PATCH] Support heterogenous lookups in mstch

Summary: Now that `mstch::render` is deleted we can slowly begin to clean up `mstch::object` :)

Reviewed By: yoney

Differential Revision: D68127190

fbshipit-source-id: 9a221b92169608c7c3a6fd3cdbc756131c83d431
---
 thrift/compiler/whisker/mstch_compat.cc | 14 ++++----------
 thrift/compiler/whisker/mstch_compat.h  | 21 ++++++++++++---------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/thrift/compiler/whisker/mstch_compat.cc b/thrift/compiler/whisker/mstch_compat.cc
index 3175e0d04c1..230d0f76c65 100644
--- a/thrift/compiler/whisker/mstch_compat.cc
+++ b/thrift/compiler/whisker/mstch_compat.cc
@@ -120,12 +120,9 @@ class mstch_map_proxy final
     if (auto cached = converted_.find(id); cached != converted_.end()) {
       return manage_derived_ref(shared_from_this(), cached->second);
     }
-    // mstch does not support heterogenous lookups, so we need a temporary
-    // std::string.
-    std::string id_string{id};
-    if (auto property = proxied_.find(id_string); property != proxied_.end()) {
+    if (auto property = proxied_.find(id); property != proxied_.end()) {
       auto [result, inserted] = converted_.insert(
-          {std::move(id_string), from_mstch(std::move(property->second))});
+          {std::string(id), from_mstch(std::move(property->second))});
       assert(inserted);
       return manage_derived_ref(shared_from_this(), result->second);
     }
@@ -187,15 +184,12 @@ class mstch_object_proxy
   }
 
   object::ptr lookup_property(std::string_view id) const override {
-    // mstch does not support heterogenous lookups, so we need a temporary
-    // std::string.
-    std::string id_string{id};
-    if (!proxied_->has(id_string)) {
+    if (!proxied_->has(id)) {
       return nullptr;
     }
 
     return detail::variant_match(
-        proxied_->at(id_string),
+        proxied_->at(id),
         [&](const mstch_node& node) -> object::ptr {
           object::ptr converted = manage_owned<object>(from_mstch(node));
           return manage_derived(shared_from_this(), std::move(converted));
diff --git a/thrift/compiler/whisker/mstch_compat.h b/thrift/compiler/whisker/mstch_compat.h
index 93d4b07f80e..25b59efc27a 100644
--- a/thrift/compiler/whisker/mstch_compat.h
+++ b/thrift/compiler/whisker/mstch_compat.h
@@ -44,18 +44,20 @@ class object_t {
   using node_ref = std::reference_wrapper<const N>;
   using lookup_result = std::variant<whisker::object::ptr, node_ref>;
 
-  lookup_result at(const std::string& name) const {
-    return methods_.at(name)();
+  lookup_result at(std::string_view name) const {
+    assert(has(name));
+    const auto& [_, do_lookup] = *methods_.find(name);
+    return do_lookup();
   }
 
-  bool has(const std::string& name) const {
-    return (methods_.find(name) != methods_.end());
+  bool has(std::string_view name) const {
+    return methods_.find(name) != methods_.end();
   }
 
   std::vector<std::string> property_names() const {
     std::vector<std::string> result;
-    for (auto& entry : methods_) {
-      result.push_back(entry.first);
+    for (const auto& [name, _] : methods_) {
+      result.push_back(name);
     }
     return result;
   }
@@ -200,7 +202,8 @@ class object_t {
     }
   }
 
-  std::unordered_map<std::string, property_dispatcher> methods_;
+  // Before C++20, std::unordered_map does not support heterogenous lookups
+  std::map<std::string, property_dispatcher, std::less<>> methods_;
 };
 
 template <typename Node>
@@ -211,7 +214,7 @@ using node_base = std::variant<
     double,
     bool,
     std::shared_ptr<internal::object_t<Node>>,
-    std::map<const std::string, Node>,
+    std::map<const std::string, Node, std::less<>>,
     std::vector<Node>>;
 
 } // namespace internal
@@ -246,7 +249,7 @@ node make_shared_node(A&&... a) {
 }
 
 using object = internal::object_t<node>;
-using map = std::map<const std::string, node>;
+using map = std::map<const std::string, node, std::less<>>;
 using array = std::vector<node>;
 
 } // namespace apache::thrift::mstch