diff --git a/thrift/compiler/whisker/object.cc b/thrift/compiler/whisker/object.cc index a8b3a7f716f..41318e93361 100644 --- a/thrift/compiler/whisker/object.cc +++ b/thrift/compiler/whisker/object.cc @@ -146,7 +146,8 @@ bool native_object::operator==(const native_object& other) const { native_function::context::array_like::array_like( native_object::array_like::ptr&& arr) : which_(std::move(arr)) {} -native_function::context::array_like::array_like(maybe_managed_ptr&& arr) +native_function::context::array_like::array_like( + managed_object_ptr&& arr) : which_(std::move(arr)) {} native_function::context::array_like::~array_like() noexcept = default; @@ -175,21 +176,20 @@ const object& native_function::context::array_like::at( [&](const native_object::array_like::ptr& arr) -> const object& { return arr->at(index); }, - [&](const maybe_managed_ptr& arr) -> const object& { + [&](const managed_object_ptr& arr) -> const object& { return (*arr)[index]; }); } native_function::context::map_like::map_like(native_object::map_like::ptr&& m) : which_(std::move(m)) {} -native_function::context::map_like::map_like(maybe_managed_ptr&& m) +native_function::context::map_like::map_like(managed_object_ptr&& m) : which_(std::move(m)) {} /* static */ std::optional native_function::context::array_like::try_from(const object::ptr& o) { if (o->is_array()) { - return array_like( - maybe_managed_ptr(o, std::addressof(o->as_array()))); + return array_like(object::derive(o, o->as_array())); } if (o->is_native_object()) { if (native_object::array_like::ptr arr = @@ -219,7 +219,7 @@ const object* native_function::context::map_like::lookup_property( [&](const native_object::map_like::ptr& m) -> const object* { return m->lookup_property(identifier); }, - [&](const maybe_managed_ptr& m) -> const object* { + [&](const managed_object_ptr& m) -> const object* { if (auto it = m->find(identifier); it != m->end()) { return &it->second; } @@ -230,7 +230,7 @@ const object* native_function::context::map_like::lookup_property( /* static */ std::optional native_function::context::map_like::try_from(const object::ptr& o) { if (o->is_map()) { - return map_like(maybe_managed_ptr(o, std::addressof(o->as_map()))); + return map_like(object::derive(o, o->as_map())); } if (o->is_native_object()) { if (native_object::map_like::ptr m = o->as_native_object()->as_map_like()) { diff --git a/thrift/compiler/whisker/object.h b/thrift/compiler/whisker/object.h index 095fce4b411..bddad6cc5bf 100644 --- a/thrift/compiler/whisker/object.h +++ b/thrift/compiler/whisker/object.h @@ -113,19 +113,18 @@ struct object_print_options { }; /** - * A shared_ptr which represents a (possibly) managed object. + * A managed_object_ptr (where T is whisker::object or any of its + * alternatives) represents one of: + * 1. A "static"-ally managed object. See object::as_static(...). + * 2. An owned instance of T. See object::owned(...). + * 3. A derived instance of T, that is a sub-object of another managed object. + * See object::derive(...). * - * 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`. + * All three cases are represented by shared_ptr. See `folly::MaybeManagedPtr`. + * (1) and (3) use the aliasing constructor of shared_ptr. */ -template -using maybe_managed_ptr = std::shared_ptr; -using object_ptr = maybe_managed_ptr; +template +using managed_object_ptr = std::shared_ptr; /** * A native_object is the most powerful type in Whisker. Its properties and @@ -156,7 +155,7 @@ class native_object { */ class map_like { public: - using ptr = maybe_managed_ptr; + using ptr = managed_object_ptr; virtual ~map_like() = default; /** * Searches for a property on an object whose name matches the provided @@ -207,7 +206,7 @@ class native_object { */ class array_like { public: - using ptr = maybe_managed_ptr; + using ptr = managed_object_ptr; virtual ~array_like() = default; /** * Returns the number of elements in the sequence. @@ -273,7 +272,7 @@ namespace detail { * argument(...) or named_argument(...). * * Small primitive types (i64, f64, boolean) are returned by value. - * The larger primitive type (string) is returned as a maybe_managed_ptr. + * The larger primitive type (string) is returned as a managed_object_ptr. * * Maps and arrays are wrapped by helper classes, in order to abstract away * differences with native_object::array_like and native_object::map_like @@ -314,7 +313,7 @@ class native_function { * ctx.declare_named_arguments({}); * i64 a = ctx.argument(0); * i64 b = ctx.argument(1); - * return object::managed(whisker::make::boolean(a == b)); + * return object::owned(whisker::make::boolean(a == b)); * } * }; * @@ -337,7 +336,7 @@ class native_function { * } * result += *ctx.argument(i); * } - * return object::managed( + * return object::owned( * whisker::make::string(std::move(result))); * } * }; @@ -352,7 +351,7 @@ class native_function { * Postconditions: * - The returned object is non-null. */ - virtual object_ptr invoke(context) = 0; + virtual managed_object_ptr<> invoke(context) = 0; /** * An exception that can be thrown to indicate a fatal error in function @@ -431,13 +430,13 @@ class native_function { * Tries to marshal the provided object into an array-like object, if the * underlying type matches. Otherwise, returns an empty optional. */ - static std::optional try_from(const object_ptr&); + static std::optional try_from(const managed_object_ptr<>&); private: explicit array_like(native_object::array_like::ptr&& arr); - explicit array_like(maybe_managed_ptr&& arr); + explicit array_like(managed_object_ptr&& arr); - std::variant> + std::variant> which_; friend class context; @@ -465,13 +464,14 @@ class native_function { * Tries to marshal the provided object into an map-like object, if the * underlying type matches. Otherwise, returns an empty optional. */ - static std::optional try_from(const object_ptr&); + static std::optional try_from(const managed_object_ptr<>&); private: explicit map_like(native_object::map_like::ptr&& m); - explicit map_like(maybe_managed_ptr&& m); + explicit map_like(managed_object_ptr&& m); - std::variant> which_; + std::variant> + which_; friend class context; }; @@ -514,7 +514,7 @@ class native_function { * If the argument is not present and presence is required, then this throws * an error. Otherwise, returns nullptr. */ - object_ptr named_argument( + managed_object_ptr<> named_argument( std::string_view name, named_argument_presence = named_argument_presence::required) const; @@ -620,7 +620,7 @@ struct native_function_argument_result { }; template <> struct native_function_argument_result { - using type = maybe_managed_ptr; + using type = managed_object_ptr; using optional_type = type; }; template <> @@ -689,28 +689,38 @@ class object final : private detail::object_base { return std::get(*this); } - using ptr = object_ptr; + using ptr = managed_object_ptr<>; /** - * Returns a shared_ptr which is an unmanaged reference to the provided + * Returns a pointer 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_static(const object& o) { + return ptr(managed_object_ptr(), &o); } - static ptr as_ref(object&&) = delete; + static ptr as_static(object&&) = delete; /** - * Returns a shared_ptr which directly owns a copy of the provided object. + * Returns a pointer which directly owns the provided object. * This allows native_function to return values that are dynamically computed. */ - static ptr managed(object&& o) { + static ptr owned(object&& o) { return std::make_shared(std::move(o)); } + /** + * Returns a pointer where `value` is known to be a sub-object of `from`. + * This means that `from` must outlive `value`. + */ + template + static managed_object_ptr derive( + const managed_object_ptr& from, const T& value) { + return managed_object_ptr(from, std::addressof(value)); + } + /* implicit */ object(null = {}) : base(std::in_place_type) {} explicit object(boolean value) : base(bool(value)) {} explicit object(i64 value) : base(value) {} @@ -976,7 +986,7 @@ native_function::context::argument(size_t index) const { error("Argument at index {} is of an unexpected type.", index); } if constexpr (std::is_same_v) { - return maybe_managed_ptr(o, std::addressof(o->as())); + return object::derive(o, o->as()); } else { return o->as(); } @@ -1014,7 +1024,7 @@ native_function::context::named_argument( error("Named argument '{}' is of an unexpected type.", name); } if constexpr (std::is_same_v) { - return maybe_managed_ptr(o, std::addressof(o->as())); + return object::derive(o, o->as()); } else { return o->as(); } diff --git a/thrift/compiler/whisker/render.cc b/thrift/compiler/whisker/render.cc index b2e448330a9..88ac7e54745 100644 --- a/thrift/compiler/whisker/render.cc +++ b/thrift/compiler/whisker/render.cc @@ -503,22 +503,22 @@ class render_engine { return detail::variant_match( expr.which, [](const expression::string_literal& s) -> object::ptr { - return object::managed(whisker::make::string(s.text)); + return object::owned(whisker::make::string(s.text)); }, [](const expression::i64_literal& i) -> object::ptr { - return object::managed(whisker::make::i64(i.value)); + return object::owned(whisker::make::i64(i.value)); }, [](const expression::null_literal&) -> object::ptr { - return object::as_ref(whisker::make::null); + return object::as_static(whisker::make::null); }, [](const expression::true_literal&) -> object::ptr { - return object::as_ref(whisker::make::true_); + return object::as_static(whisker::make::true_); }, [](const expression::false_literal&) -> object::ptr { - return object::as_ref(whisker::make::false_); + return object::as_static(whisker::make::false_); }, [&](const ast::variable_lookup& variable_lookup) -> object::ptr { - return object::as_ref(lookup_variable(variable_lookup)); + return object::as_static(lookup_variable(variable_lookup)); }, [&](const function_call& func) -> object::ptr { return detail::variant_match( @@ -528,28 +528,28 @@ class render_engine { assert(func.positional_arguments.size() == 1); assert(func.named_arguments.empty()); return evaluate_as_bool(func.positional_arguments[0]) - ? object::as_ref(whisker::make::false_) - : object::as_ref(whisker::make::true_); + ? object::as_static(whisker::make::false_) + : object::as_static(whisker::make::true_); }, [&](function_call::builtin_and) -> object::ptr { // enforced by the parser assert(func.named_arguments.empty()); for (const expression& arg : func.positional_arguments) { if (!evaluate_as_bool(arg)) { - return object::as_ref(whisker::make::false_); + return object::as_static(whisker::make::false_); } } - return object::as_ref(whisker::make::true_); + return object::as_static(whisker::make::true_); }, [&](function_call::builtin_or) -> object::ptr { // enforced by the parser assert(func.named_arguments.empty()); for (const expression& arg : func.positional_arguments) { if (evaluate_as_bool(arg)) { - return object::as_ref(whisker::make::true_); + return object::as_static(whisker::make::true_); } } - return object::as_ref(whisker::make::false_); + return object::as_static(whisker::make::false_); }, [&](const function_call::user_defined& user_defined) -> object::ptr { diff --git a/thrift/compiler/whisker/test/render_test.cc b/thrift/compiler/whisker/test/render_test.cc index 1cbe2f5d10c..32630c0f86e 100644 --- a/thrift/compiler/whisker/test/render_test.cc +++ b/thrift/compiler/whisker/test/render_test.cc @@ -712,7 +712,7 @@ class add : public native_function { for (std::size_t i = 0; i < ctx.arity(); ++i) { result += ctx.argument(i); } - return object::managed(w::i64(negate ? -result : result)); + return object::owned(w::i64(negate ? -result : result)); } }; @@ -725,7 +725,7 @@ class i64_eq : public native_function { ctx.declare_named_arguments({}); i64 a = ctx.argument(0); i64 b = ctx.argument(1); - return object::managed(w::boolean(a == b)); + return object::owned(w::boolean(a == b)); } }; @@ -747,7 +747,7 @@ class str_concat : public native_function { } result += *ctx.argument(i); } - return object::managed(w::string(std::move(result))); + return object::owned(w::string(std::move(result))); } }; @@ -759,7 +759,7 @@ class array_len : public native_function { ctx.declare_arity(1); ctx.declare_named_arguments({}); auto len = i64(ctx.argument(0).size()); - return object::managed(w::i64(len)); + return object::owned(w::i64(len)); } }; @@ -775,7 +775,7 @@ class map_get : public native_function { auto key = ctx.named_argument("key", context::required); if (const object* result = m.lookup_property(*key)) { - return object::as_ref(*result); + return object::as_static(*result); } ctx.error("Key '{}' not found.", *key); } @@ -959,7 +959,7 @@ TEST_F(RenderTest, user_defined_function_array_like_named_argument) { ctx.declare_arity(0); ctx.declare_named_arguments({"input"}); auto len = i64(ctx.named_argument("input")->size()); - return object::managed(w::string(fmt::format("length is {}", len))); + return object::owned(w::string(fmt::format("length is {}", len))); } }; @@ -1041,8 +1041,7 @@ TEST_F(RenderTest, user_defined_function_map_like_named_argument) { std::string_view result = m->lookup_property(*key) == nullptr ? "missing" : "present"; - return object::managed( - w::string(fmt::format("map element is {}", result))); + return object::owned(w::string(fmt::format("map element is {}", result))); } };