Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GDExtension: Use Uninitialized*Ptr for constructors/converters using placement new #78419

Merged
merged 1 commit into from Jun 19, 2023

Conversation

Bromeon
Copy link
Contributor

@Bromeon Bromeon commented Jun 18, 2023

A few methods in the GDExtension header construct their destination in-place, but the parameters are not declared using the Uninitialized* pointer types.

See godot-cpp PR: godotengine/godot-cpp#1148


Below are the relevant sections where placement new is used.

TypePtr constructors:

template <class T>
struct PtrConstruct {};
#define MAKE_PTRCONSTRUCT(m_type) \
template <> \
struct PtrConstruct<m_type> { \
_FORCE_INLINE_ static void construct(const m_type &p_value, void *p_ptr) { \
memnew_placement(p_ptr, m_type(p_value)); \
} \
};

VariantPtr <-> TypePtr conversions:

template <class T>
struct VariantTypeConstructor {
_FORCE_INLINE_ static void variant_from_type(void *r_variant, void *p_value) {
// r_variant is provided by caller as uninitialized memory
memnew_placement(r_variant, Variant(*((T *)p_value)));
}
_FORCE_INLINE_ static void type_from_variant(void *r_value, void *p_variant) {
// r_value is provided by caller as uninitialized memory
memnew_placement(r_value, T(VariantInternalAccessor<T>::get(reinterpret_cast<Variant *>(p_variant))));
}
};

…acement new

These methods construct their destination in-place, but the parameters are not declared using the `Uninitialized*Ptr` pointer types.
Copy link
Member

@touilleMan touilleMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

O_o I'm very puzzled how I missed that (especially given VariantPtr <-> TypePtr with uninitialized input was introduced in my PR !)

Anyway great job 🥳 🎉

@akien-mga akien-mga added this to the 4.1 milestone Jun 19, 2023
@akien-mga akien-mga merged commit 914cc1d into godotengine:master Jun 19, 2023
13 checks passed
@akien-mga
Copy link
Member

Thanks!

@Bromeon Bromeon deleted the bugfix/uninit-ptr-signature branch June 19, 2023 10:29
@YuriSizov YuriSizov changed the title GDExtension: Uninitialized*Ptr for constructors/converters using placement new GDExtension: Use Uninitialized*Ptr for constructors/converters using placement new Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants