diff --git a/thrift/compiler/whisker/eval_context.cc b/thrift/compiler/whisker/eval_context.cc index b769f51c972..0b61b59e749 100644 --- a/thrift/compiler/whisker/eval_context.cc +++ b/thrift/compiler/whisker/eval_context.cc @@ -66,7 +66,7 @@ class global_scope_object explicit global_scope_object(map properties) : properties_(std::move(properties)) {} - std::shared_ptr as_map_like() const override { + native_object::map_like::ptr as_map_like() const override { return shared_from_this(); } diff --git a/thrift/compiler/whisker/mstch_compat.cc b/thrift/compiler/whisker/mstch_compat.cc index 7510db63786..1d6f776de20 100644 --- a/thrift/compiler/whisker/mstch_compat.cc +++ b/thrift/compiler/whisker/mstch_compat.cc @@ -42,8 +42,7 @@ class mstch_array_proxy final : proxied_(std::move(array)) {} private: - std::shared_ptr as_array_like() - const override { + native_object::array_like::ptr as_array_like() const override { return shared_from_this(); } @@ -106,7 +105,7 @@ class mstch_map_proxy final explicit mstch_map_proxy(mstch_map&& map) : proxied_(std::move(map)) {} private: - std::shared_ptr as_map_like() const override { + native_object::map_like::ptr as_map_like() const override { return shared_from_this(); } @@ -176,7 +175,7 @@ class mstch_object_proxy explicit mstch_object_proxy(std::shared_ptr&& obj) : proxied_(std::move(obj)) {} - std::shared_ptr as_map_like() const override { + native_object::map_like::ptr as_map_like() const override { return shared_from_this(); } diff --git a/thrift/compiler/whisker/object.h b/thrift/compiler/whisker/object.h index 42ca35cc281..d6beb9626c1 100644 --- a/thrift/compiler/whisker/object.h +++ b/thrift/compiler/whisker/object.h @@ -104,6 +104,20 @@ struct object_print_options { unsigned max_depth = std::numeric_limits::max(); }; +/** + * A shared_ptr which represents a (possibly) managed object. + * + * When storing a raw pointer, maybe_managed_ptr does not manage the pointer's + * lifetime, i.e. never calls `free` on the pointer. + * + * When storing a shared_ptr, maybe_managed_ptr will release the shared_ptr + * upon its own destruction. + * + * See `folly::MaybeManagedPtr`. + */ +template +using maybe_managed_ptr = std::shared_ptr; + /** * A native_object is the most powerful type in Whisker. Its properties and * behavior are defined by highly customizable C++ code. @@ -133,6 +147,7 @@ class native_object { */ class map_like { public: + using ptr = maybe_managed_ptr; virtual ~map_like() = default; /** * Searches for a property on an object whose name matches the provided @@ -168,17 +183,14 @@ class native_object { * public native_object::map_like, * public std::enable_shared_from_this { * public: - * std::shared_ptr as_map_like() - * const override { + * native_object::map_like:ptr as_map_like() const override { * return shared_from_this(); * } * * // Implement map-like functions... * }; */ - virtual std::shared_ptr as_map_like() const { - return nullptr; - } + virtual native_object::map_like::ptr as_map_like() const { return nullptr; } /** * A class that allows "array-like" random access over an underlying sequence @@ -186,6 +198,7 @@ class native_object { */ class array_like { public: + using ptr = maybe_managed_ptr; virtual ~array_like() = default; /** * Returns the number of elements in the sequence. @@ -214,16 +227,14 @@ class native_object { * public native_object::array_like, * public std::enable_shared_from_this { * public: - * std::shared_ptr as_array_like() - * const override { + * native_object::array_like::ptr as_array_like() const override { * return shared_from_this(); * } * * // Implement array-like functions... * }; */ - virtual std::shared_ptr as_array_like() - const { + virtual native_object::array_like::ptr as_array_like() const { return nullptr; } @@ -311,6 +322,28 @@ class object final : private detail::object_base { base&& steal_variant() & { return std::move(*this); } public: + using ptr = maybe_managed_ptr; + + /** + * Returns a shared_ptr which is an unmanaged reference to the provided + * object. + * + * The caller must guarantee that underlying object is kept alive for the + * duration of an expression evaluation in which the object is used. + */ + static ptr as_ref(const object& o) { + return ptr(std::shared_ptr(), &o); + } + static ptr as_ref(object&&) = delete; + + /** + * Returns a shared_ptr which directly owns a copy of the provided object. + * This allows native_function to return values that are dynamically computed. + */ + static ptr managed(object&& o) { + return std::make_shared(std::move(o)); + } + /* implicit */ object(null = {}) : base(std::in_place_type) {} explicit object(boolean value) : base(bool(value)) {} explicit object(i64 value) : base(value) {} diff --git a/thrift/compiler/whisker/render.cc b/thrift/compiler/whisker/render.cc index 8fed44fc016..54b75664b32 100644 --- a/thrift/compiler/whisker/render.cc +++ b/thrift/compiler/whisker/render.cc @@ -341,67 +341,61 @@ class render_engine { }); } - std::shared_ptr evaluate(const ast::expression& expr) { - using object_ptr = std::shared_ptr; - static const auto as_ref = [](const object& o) -> object_ptr { - // Empty deleter here implies that the object is externally managed. - // Thus, the returned shared_ptr acts like a reference. - return object_ptr(std::shared_ptr(), &o); - }; - - using function_call = ast::expression::function_call; + object::ptr evaluate(const ast::expression& expr) { + using expression = ast::expression; + using function_call = expression::function_call; return detail::variant_match( expr.which, - [](const ast::expression::string_literal& s) -> object_ptr { - return std::make_shared(string(s.text)); + [](const expression::string_literal& s) -> object::ptr { + return object::managed(whisker::make::string(s.text)); }, - [](const ast::expression::i64_literal& i) -> object_ptr { - return std::make_shared(i64(i.value)); + [](const expression::i64_literal& i) -> object::ptr { + return object::managed(whisker::make::i64(i.value)); }, - [](const ast::expression::null_literal&) -> object_ptr { - return as_ref(whisker::make::null); + [](const expression::null_literal&) -> object::ptr { + return object::as_ref(whisker::make::null); }, - [](const ast::expression::true_literal&) -> object_ptr { - return as_ref(whisker::make::true_); + [](const expression::true_literal&) -> object::ptr { + return object::as_ref(whisker::make::true_); }, - [](const ast::expression::false_literal&) -> object_ptr { - return as_ref(whisker::make::false_); + [](const expression::false_literal&) -> object::ptr { + return object::as_ref(whisker::make::false_); }, - [&](const ast::variable_lookup& variable_lookup) -> object_ptr { - return as_ref(lookup_variable(variable_lookup)); + [&](const ast::variable_lookup& variable_lookup) -> object::ptr { + return object::as_ref(lookup_variable(variable_lookup)); }, - [&](const function_call& func) -> object_ptr { + [&](const function_call& func) -> object::ptr { return detail::variant_match( func.which, - [&](function_call::builtin_not) -> object_ptr { + [&](function_call::builtin_not) -> object::ptr { // enforced by the parser assert(func.positional_arguments.size() == 1); assert(func.named_arguments.empty()); return evaluate_as_bool(func.positional_arguments[0]) - ? as_ref(whisker::make::false_) - : as_ref(whisker::make::true_); + ? object::as_ref(whisker::make::false_) + : object::as_ref(whisker::make::true_); }, - [&](function_call::builtin_and) -> object_ptr { + [&](function_call::builtin_and) -> object::ptr { // enforced by the parser assert(func.named_arguments.empty()); - for (const ast::expression& arg : func.positional_arguments) { + for (const expression& arg : func.positional_arguments) { if (!evaluate_as_bool(arg)) { - return as_ref(whisker::make::false_); + return object::as_ref(whisker::make::false_); } } - return as_ref(whisker::make::true_); + return object::as_ref(whisker::make::true_); }, - [&](function_call::builtin_or) -> object_ptr { + [&](function_call::builtin_or) -> object::ptr { // enforced by the parser assert(func.named_arguments.empty()); - for (const ast::expression& arg : func.positional_arguments) { + for (const expression& arg : func.positional_arguments) { if (evaluate_as_bool(arg)) { - return as_ref(whisker::make::true_); + return object::as_ref(whisker::make::true_); } } - return as_ref(whisker::make::false_); + return object::as_ref(whisker::make::false_); }, - [&](const function_call::user_defined& f) -> object_ptr { + [&](const function_call::user_defined& f) -> object::ptr { diags_.error( f.name.loc.begin, "User-defined function '{}' not supported yet.", @@ -437,7 +431,7 @@ class render_engine { } void visit(const ast::interpolation& interpolation) { - std::shared_ptr result = evaluate(interpolation.content); + object::ptr result = evaluate(interpolation.content); const auto report_unprintable_message_only = [&](diagnostic_level level) { maybe_report(interpolation.loc, level, [&] { @@ -500,7 +494,7 @@ class render_engine { } } bool evaluate_as_bool(const ast::expression& expr) { - std::shared_ptr result = evaluate(expr); + object::ptr result = evaluate(expr); return result->visit( [&](boolean value) { return value; }, [&](const auto& value) { @@ -654,7 +648,7 @@ class render_engine { void visit(const ast::with_block& with_block) { const ast::expression& expr = with_block.value; - std::shared_ptr result = evaluate(expr); + object::ptr result = evaluate(expr); result->visit( [&](const map&) { // maps can be de-structured. diff --git a/thrift/compiler/whisker/test/eval_context_test.cc b/thrift/compiler/whisker/test/eval_context_test.cc index 63027891b57..b59b520c9e1 100644 --- a/thrift/compiler/whisker/test/eval_context_test.cc +++ b/thrift/compiler/whisker/test/eval_context_test.cc @@ -43,7 +43,7 @@ class double_property_name public native_object::map_like, public std::enable_shared_from_this { public: - std::shared_ptr as_map_like() const override { + native_object::map_like::ptr as_map_like() const override { return shared_from_this(); } @@ -72,7 +72,7 @@ class delegate_to : public native_object, : delegate_(std::move(delegate)) {} private: - std::shared_ptr as_map_like() const override { + native_object::map_like::ptr as_map_like() const override { return shared_from_this(); } diff --git a/thrift/compiler/whisker/test/render_test.cc b/thrift/compiler/whisker/test/render_test.cc index 77a0c3e3b38..c907cbf3392 100644 --- a/thrift/compiler/whisker/test/render_test.cc +++ b/thrift/compiler/whisker/test/render_test.cc @@ -38,7 +38,7 @@ class double_property_name public native_object::map_like, public std::enable_shared_from_this { public: - std::shared_ptr as_map_like() const override { + native_object::map_like::ptr as_map_like() const override { return shared_from_this(); } @@ -204,8 +204,7 @@ TEST_F(RenderTest, section_block_array_iterable_native_object) { explicit array_like_native_object(array values) : values_(std::move(values)) {} - std::shared_ptr as_array_like() - const override { + native_object::array_like::ptr as_array_like() const override { return shared_from_this(); } std::size_t size() const override { return values_.size(); } @@ -294,8 +293,7 @@ TEST_F(RenderTest, section_block_map_like_native_object) { public: explicit map_like_native_object(map values) : values_(std::move(values)) {} - std::shared_ptr as_map_like() - const override { + native_object::map_like::ptr as_map_like() const override { return shared_from_this(); }