Skip to content

Commit

Permalink
Support heterogenous lookups in mstch
Browse files Browse the repository at this point in the history
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
  • Loading branch information
praihan authored and facebook-github-bot committed Jan 16, 2025
1 parent f9af23e commit 39d71b1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 19 deletions.
14 changes: 4 additions & 10 deletions thrift/compiler/whisker/mstch_compat.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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));
Expand Down
21 changes: 12 additions & 9 deletions thrift/compiler/whisker/mstch_compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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>
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 39d71b1

Please sign in to comment.