From d95d49ee124abfd2c0befde69821a78d9035c421 Mon Sep 17 00:00:00 2001 From: David Snopek Date: Wed, 6 Dec 2023 14:20:11 -0600 Subject: [PATCH] Add `RequiredParam` and `RequiredValue` to mark `Object *` arguments and return values as required Co-authored-by: Thaddeus Crews --- core/extension/extension_api_dump.cpp | 2 +- core/extension/gdextension_interface.json | 4 + core/object/object.h | 1 + core/variant/binder_common.h | 1 + core/variant/method_ptrcall.h | 36 ++++ core/variant/required_ptr.h | 246 ++++++++++++++++++++++ core/variant/type_info.h | 39 ++++ core/variant/variant_internal.h | 24 +++ tests/core/object/test_object.h | 24 +++ 9 files changed, 376 insertions(+), 1 deletion(-) create mode 100644 core/variant/required_ptr.h diff --git a/core/extension/extension_api_dump.cpp b/core/extension/extension_api_dump.cpp index 62b605f7d30..4d7d82e118d 100644 --- a/core/extension/extension_api_dump.cpp +++ b/core/extension/extension_api_dump.cpp @@ -88,7 +88,7 @@ static String get_property_info_type_name(const PropertyInfo &p_info) { } static String get_type_meta_name(const GodotTypeInfo::Metadata metadata) { - static const char *argmeta[13] = { "none", "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64", "float", "double", "char16", "char32" }; + static const char *argmeta[14] = { "none", "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64", "float", "double", "char16", "char32", "required" }; return argmeta[metadata]; } diff --git a/core/extension/gdextension_interface.json b/core/extension/gdextension_interface.json index 251d8dba789..8639b30ddea 100644 --- a/core/extension/gdextension_interface.json +++ b/core/extension/gdextension_interface.json @@ -1942,6 +1942,10 @@ { "name": "GDEXTENSION_METHOD_ARGUMENT_METADATA_INT_IS_CHAR32", "value": 12 + }, + { + "name": "GDEXTENSION_METHOD_ARGUMENT_METADATA_OBJECT_IS_REQUIRED", + "value": 13 } ] }, diff --git a/core/object/object.h b/core/object/object.h index aeb8c752a06..19934316519 100644 --- a/core/object/object.h +++ b/core/object/object.h @@ -39,6 +39,7 @@ #include "core/templates/hash_set.h" #include "core/templates/list.h" #include "core/templates/safe_refcount.h" +#include "core/variant/required_ptr.h" #include "core/variant/variant.h" template diff --git a/core/variant/binder_common.h b/core/variant/binder_common.h index 0b8f0000fb9..08eb38eaac9 100644 --- a/core/variant/binder_common.h +++ b/core/variant/binder_common.h @@ -36,6 +36,7 @@ #include "core/templates/simple_type.h" #include "core/typedefs.h" #include "core/variant/method_ptrcall.h" +#include "core/variant/required_ptr.h" #include "core/variant/type_info.h" #include "core/variant/variant.h" #include "core/variant/variant_internal.h" diff --git a/core/variant/method_ptrcall.h b/core/variant/method_ptrcall.h index 886b1facb20..a1a697dacc8 100644 --- a/core/variant/method_ptrcall.h +++ b/core/variant/method_ptrcall.h @@ -269,6 +269,42 @@ struct PtrToArg { } }; +// This is for RequiredParam. + +template +struct PtrToArg> { + typedef typename RequiredParam::ptr_type EncodeT; + + _FORCE_INLINE_ static RequiredParam convert(const void *p_ptr) { + if (p_ptr == nullptr) { + return RequiredParam::_err_return_dont_use(); + } + return RequiredParam(*reinterpret_cast(p_ptr)); + } + + _FORCE_INLINE_ static void encode(const RequiredParam &p_var, void *p_ptr) { + *((typename RequiredParam::ptr_type *)p_ptr) = p_var._internal_ptr_dont_use(); + } +}; + +// This is for RequiredResult. + +template +struct PtrToArg> { + typedef typename RequiredResult::ptr_type EncodeT; + + _FORCE_INLINE_ static RequiredResult convert(const void *p_ptr) { + if (p_ptr == nullptr) { + return RequiredResult::_err_return_dont_use(); + } + return RequiredResult(*reinterpret_cast(p_ptr)); + } + + _FORCE_INLINE_ static void encode(const RequiredResult &p_var, void *p_ptr) { + *((typename RequiredResult::ptr_type *)p_ptr) = p_var._internal_ptr_dont_use(); + } +}; + // This is for ObjectID. template <> diff --git a/core/variant/required_ptr.h b/core/variant/required_ptr.h new file mode 100644 index 00000000000..5d75862ec0e --- /dev/null +++ b/core/variant/required_ptr.h @@ -0,0 +1,246 @@ +/**************************************************************************/ +/* required_ptr.h */ +/**************************************************************************/ +/* This file is part of: */ +/* GODOT ENGINE */ +/* https://godotengine.org */ +/**************************************************************************/ +/* Copyright (c) 2014-present Godot Engine contributors (see AUTHORS.md). */ +/* Copyright (c) 2007-2014 Juan Linietsky, Ariel Manzur. */ +/* */ +/* Permission is hereby granted, free of charge, to any person obtaining */ +/* a copy of this software and associated documentation files (the */ +/* "Software"), to deal in the Software without restriction, including */ +/* without limitation the rights to use, copy, modify, merge, publish, */ +/* distribute, sublicense, and/or sell copies of the Software, and to */ +/* permit persons to whom the Software is furnished to do so, subject to */ +/* the following conditions: */ +/* */ +/* The above copyright notice and this permission notice shall be */ +/* included in all copies or substantial portions of the Software. */ +/* */ +/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, */ +/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF */ +/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. */ +/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY */ +/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, */ +/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE */ +/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +/**************************************************************************/ + +#pragma once + +#include "core/variant/variant.h" + +template +class RequiredResult { + static_assert(!is_fully_defined_v || std::is_base_of_v, "T must be an Object subtype"); + +public: + using element_type = T; + using ptr_type = std::conditional_t, Ref, T *>; + +private: + ptr_type _value = ptr_type(); + + _FORCE_INLINE_ RequiredResult() = default; + +public: + RequiredResult(const RequiredResult &p_other) = default; + RequiredResult(RequiredResult &&p_other) = default; + RequiredResult &operator=(const RequiredResult &p_other) = default; + RequiredResult &operator=(RequiredResult &&p_other) = default; + + _FORCE_INLINE_ RequiredResult(std::nullptr_t) : + RequiredResult() {} + _FORCE_INLINE_ RequiredResult &operator=(std::nullptr_t) { _value = nullptr; } + + // These functions should not be called directly, they are only for internal use. + _FORCE_INLINE_ ptr_type _internal_ptr_dont_use() const { return _value; } + _FORCE_INLINE_ static RequiredResult _err_return_dont_use() { return RequiredResult(); } + + template , int> = 0> + _FORCE_INLINE_ RequiredResult(const RequiredResult &p_other) : + _value(p_other._value) {} + template , int> = 0> + _FORCE_INLINE_ RequiredResult &operator=(const RequiredResult &p_other) { + _value = p_other._value; + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredResult(T_Other *p_ptr) : + _value(p_ptr) {} + template , int> = 0> + _FORCE_INLINE_ RequiredResult &operator=(T_Other *p_ptr) { + _value = p_ptr; + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredResult(const Ref &p_ref) : + _value(p_ref) {} + template , int> = 0> + _FORCE_INLINE_ RequiredResult &operator=(const Ref &p_ref) { + _value = p_ref; + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredResult(Ref &&p_ref) : + _value(std::move(p_ref)) {} + template , int> = 0> + _FORCE_INLINE_ RequiredResult &operator=(Ref &&p_ref) { + _value = std::move(p_ref); + return &this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredResult(const Variant &p_variant) : + _value(static_cast(p_variant.get_validated_object())) {} + template , int> = 0> + _FORCE_INLINE_ RequiredResult &operator=(const Variant &p_variant) { + _value = static_cast(p_variant.get_validated_object()); + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredResult(const Variant &p_variant) : + _value(static_cast(p_variant.operator Object *())) {} + template , int> = 0> + _FORCE_INLINE_ RequiredResult &operator=(const Variant &p_variant) { + _value = static_cast(p_variant.operator Object *()); + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ element_type *ptr() const { + return *_value; + } + + template , int> = 0> + _FORCE_INLINE_ element_type *ptr() const { + return _value; + } + + _FORCE_INLINE_ operator ptr_type() { + return _value; + } + + _FORCE_INLINE_ operator Variant() const { + return Variant(_value); + } + + _FORCE_INLINE_ element_type *operator*() const { + return ptr(); + } + + _FORCE_INLINE_ element_type *operator->() const { + return ptr(); + } +}; + +template +class RequiredParam { + static_assert(!is_fully_defined_v || std::is_base_of_v, "T must be an Object subtype"); + +public: + using element_type = T; + using ptr_type = std::conditional_t, Ref, T *>; + +private: + ptr_type _value = ptr_type(); + + _FORCE_INLINE_ RequiredParam() = default; + +public: + // These functions should not be called directly, they are only for internal use. + _FORCE_INLINE_ ptr_type _internal_ptr_dont_use() const { return _value; } + _FORCE_INLINE_ bool _is_null_dont_use() const { + if constexpr (std::is_base_of_v) { + return _value.is_null(); + } else { + return _value == nullptr; + } + } + _FORCE_INLINE_ static RequiredParam _err_return_dont_use() { return RequiredParam(); } + + // Prevent erroneously assigning null values by explicitly removing nullptr constructor/assignment. + RequiredParam(std::nullptr_t) = delete; + RequiredParam &operator=(std::nullptr_t) = delete; + + RequiredParam(const RequiredParam &p_other) = default; + RequiredParam(RequiredParam &&p_other) = default; + RequiredParam &operator=(const RequiredParam &p_other) = default; + RequiredParam &operator=(RequiredParam &&p_other) = default; + + template , int> = 0> + _FORCE_INLINE_ RequiredParam(const RequiredParam &p_other) : + _value(p_other._internal_ptr_dont_use()) {} + template , int> = 0> + _FORCE_INLINE_ RequiredParam &operator=(const RequiredParam &p_other) { + _value = p_other._internal_ptr_dont_use(); + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredParam(T_Other *p_ptr) : + _value(p_ptr) {} + template , int> = 0> + _FORCE_INLINE_ RequiredParam &operator=(T_Other *p_ptr) { + _value = p_ptr; + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredParam(const Ref &p_ref) : + _value(p_ref) {} + template , int> = 0> + _FORCE_INLINE_ RequiredParam &operator=(const Ref &p_ref) { + _value = p_ref; + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredParam(Ref &&p_ref) : + _value(std::move(p_ref)) {} + template , int> = 0> + _FORCE_INLINE_ RequiredParam &operator=(Ref &&p_ref) { + _value = std::move(p_ref); + return &this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredParam(const Variant &p_variant) : + _value(static_cast(p_variant.get_validated_object())) {} + template , int> = 0> + _FORCE_INLINE_ RequiredParam &operator=(const Variant &p_variant) { + _value = static_cast(p_variant.get_validated_object()); + return *this; + } + + template , int> = 0> + _FORCE_INLINE_ RequiredParam(const Variant &p_variant) : + _value(static_cast(p_variant.operator Object *())) {} + template , int> = 0> + _FORCE_INLINE_ RequiredParam &operator=(const Variant &p_variant) { + _value = static_cast(p_variant.operator Object *()); + return *this; + } +}; + +#define TMPL_EXTRACT_PARAM_OR_FAIL(m_name, m_param, m_retval, m_msg, m_editor) \ + if (unlikely(m_param._is_null_dont_use())) { \ + _err_print_error(FUNCTION_STR, __FILE__, __LINE__, "Required object \"" _STR(m_param) "\" is null.", m_msg, m_editor); \ + return m_retval; \ + } \ + typename std::decay_t::ptr_type m_name = m_param._internal_ptr_dont_use(); \ + static_assert(true) + +// These macros are equivalent to the ERR_FAIL_NULL*() family of macros, only for RequiredParam instead of raw pointers. +#define EXTRACT_PARAM_OR_FAIL(m_name, m_param) TMPL_EXTRACT_PARAM_OR_FAIL(m_name, m_param, void(), "", false) +#define EXTRACT_PARAM_OR_FAIL_MSG(m_name, m_param, m_msg) TMPL_EXTRACT_PARAM_OR_FAIL(m_name, m_param, void(), m_msg, false) +#define EXTRACT_PARAM_OR_FAIL_EDMSG(m_name, m_param, m_msg) TMPL_EXTRACT_PARAM_OR_FAIL(m_name, m_param, void(), m_msg, true) +#define EXTRACT_PARAM_OR_FAIL_V(m_name, m_param, m_retval) TMPL_EXTRACT_PARAM_OR_FAIL(m_name, m_param, m_retval, "", false) +#define EXTRACT_PARAM_OR_FAIL_V_MSG(m_name, m_param, m_retval, m_msg) TMPL_EXTRACT_PARAM_OR_FAIL(m_name, m_param, m_retval, m_msg, false) +#define EXTRACT_PARAM_OR_FAIL_V_EDMSG(m_name, m_param, m_retval, m_msg) TMPL_EXTRACT_PARAM_OR_FAIL(m_name, m_param, m_retval, m_msg, true) diff --git a/core/variant/type_info.h b/core/variant/type_info.h index c52885e7131..f137ee23ce5 100644 --- a/core/variant/type_info.h +++ b/core/variant/type_info.h @@ -52,6 +52,7 @@ enum Metadata { METADATA_REAL_IS_DOUBLE, METADATA_INT_IS_CHAR16, METADATA_INT_IS_CHAR32, + METADATA_OBJECT_IS_REQUIRED, }; } @@ -182,6 +183,44 @@ struct GetTypeInfo>> { } }; +template +class RequiredParam; + +template +class RequiredResult; + +template +struct GetTypeInfo, std::enable_if_t>> { + static const Variant::Type VARIANT_TYPE = Variant::OBJECT; + static const GodotTypeInfo::Metadata METADATA = GodotTypeInfo::METADATA_OBJECT_IS_REQUIRED; + + template , int> = 0> + static inline PropertyInfo get_class_info() { + return PropertyInfo(Variant::OBJECT, String(), PROPERTY_HINT_RESOURCE_TYPE, T::get_class_static()); + } + + template , int> = 0> + static inline PropertyInfo get_class_info() { + return PropertyInfo(StringName(T::get_class_static())); + } +}; + +template +struct GetTypeInfo, std::enable_if_t>> { + static const Variant::Type VARIANT_TYPE = Variant::OBJECT; + static const GodotTypeInfo::Metadata METADATA = GodotTypeInfo::METADATA_OBJECT_IS_REQUIRED; + + template , int> = 0> + static inline PropertyInfo get_class_info() { + return PropertyInfo(Variant::OBJECT, String(), PROPERTY_HINT_RESOURCE_TYPE, T::get_class_static()); + } + + template , int> = 0> + static inline PropertyInfo get_class_info() { + return PropertyInfo(StringName(T::get_class_static())); + } +}; + namespace GodotTypeInfo { namespace Internal { inline String enum_qualified_name_to_class_info_name(const String &p_qualified_name) { diff --git a/core/variant/variant_internal.h b/core/variant/variant_internal.h index 3fe5f553066..4d807ff8702 100644 --- a/core/variant/variant_internal.h +++ b/core/variant/variant_internal.h @@ -608,6 +608,30 @@ struct VariantInternalAccessor { static _FORCE_INLINE_ void set(Variant *v, const Object *p_value) { VariantInternal::object_assign(v, p_value); } }; +template +struct VariantInternalAccessor> { + static _FORCE_INLINE_ RequiredParam get(const Variant *v) { return RequiredParam(Object::cast_to(const_cast(*VariantInternal::get_object(v)))); } + static _FORCE_INLINE_ void set(Variant *v, const RequiredParam &p_value) { VariantInternal::object_assign(v, p_value.ptr()); } +}; + +template +struct VariantInternalAccessor &> { + static _FORCE_INLINE_ RequiredParam get(const Variant *v) { return RequiredParam(Object::cast_to(*VariantInternal::get_object(v))); } + static _FORCE_INLINE_ void set(Variant *v, const RequiredParam &p_value) { VariantInternal::object_assign(v, p_value.ptr()); } +}; + +template +struct VariantInternalAccessor> { + static _FORCE_INLINE_ RequiredResult get(const Variant *v) { return RequiredResult(Object::cast_to(const_cast(*VariantInternal::get_object(v)))); } + static _FORCE_INLINE_ void set(Variant *v, const RequiredResult &p_value) { VariantInternal::object_assign(v, p_value.ptr()); } +}; + +template +struct VariantInternalAccessor &> { + static _FORCE_INLINE_ RequiredResult get(const Variant *v) { return RequiredResult(Object::cast_to(*VariantInternal::get_object(v))); } + static _FORCE_INLINE_ void set(Variant *v, const RequiredResult &p_value) { VariantInternal::object_assign(v, p_value.ptr()); } +}; + template <> struct VariantInternalAccessor { static _FORCE_INLINE_ Variant &get(Variant *v) { return *v; } diff --git a/tests/core/object/test_object.h b/tests/core/object/test_object.h index be98990884f..cf95d1b9191 100644 --- a/tests/core/object/test_object.h +++ b/tests/core/object/test_object.h @@ -596,4 +596,28 @@ TEST_CASE("[Object] Destruction at the end of the call chain is safe") { "Object was tail-deleted without crashes."); } +int required_param_compare(const Ref &p_ref, const RequiredParam &p_required) { + EXTRACT_PARAM_OR_FAIL_V(extract, p_required, false); + ERR_FAIL_COND_V(p_ref->get_reference_count() != extract->get_reference_count(), -1); + return p_ref->get_reference_count(); +} + +TEST_CASE("[Object] RequiredParam Ref") { + Ref ref; + ref.instantiate(); + + RequiredParam required = ref; + EXTRACT_PARAM_OR_FAIL(extract, required); + + static_assert(std::is_same_v); + + CHECK_EQ(ref->get_reference_count(), extract->get_reference_count()); + + const int count = required_param_compare(ref, ref); + CHECK_NE(count, -1); + CHECK_NE(count, ref->get_reference_count()); + + CHECK_EQ(ref->get_reference_count(), extract->get_reference_count()); +} + } // namespace TestObject