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

fix OpenBSD build error: implicit instantiation of undefined template… #83804

Closed
wants to merge 2 commits into from

Conversation

rfht
Copy link
Contributor

@rfht rfht commented Oct 23, 2023

… 'GetTypeInfo'

I'm not sure why OpenBSD build needs GetTypeInfo<unsigned long> but apparently this isn't an issue on other platforms. This diff supplies this, with basically the same content as for GetTypeInfo<ObjectID> which appears to consist of a UINT64.

Would like feedback on if I'm missing anything here. I'm able to complete the build with this in place.

@rfht rfht requested a review from a team as a code owner October 23, 2023 00:14
@@ -129,6 +129,9 @@ MAKE_TYPE_INFO_WITH_META(int8_t, Variant::INT, GodotTypeInfo::METADATA_INT_IS_IN
MAKE_TYPE_INFO_WITH_META(uint16_t, Variant::INT, GodotTypeInfo::METADATA_INT_IS_UINT16)
MAKE_TYPE_INFO_WITH_META(int16_t, Variant::INT, GodotTypeInfo::METADATA_INT_IS_INT16)
MAKE_TYPE_INFO_WITH_META(uint32_t, Variant::INT, GodotTypeInfo::METADATA_INT_IS_UINT32)
#ifdef __OpenBSD__
MAKE_TYPE_INFO_WITH_META(unsigned long, Variant::INT, GodotTypeInfo::METADATA_INT_IS_UINT32)
Copy link
Member

Choose a reason for hiding this comment

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

This might result in duplicate template errors (should be checked on both 32-bit and 64-bit builds).

Usually, this type of errors can be fixed by simply adding uint64_t cast to the place where ambiguous int is assigned to the Variant variable.

@rfht
Copy link
Contributor Author

rfht commented Oct 25, 2023

it looks like we have found where to use uint64_t (or rather @omar-polo has): jasperla/openbsd-wip@596d48a ... therefore closing this issue here which isn't needed and doesn't seem to be the right approach for the fix. Thanks!

@rfht rfht closed this Oct 25, 2023
@akien-mga
Copy link
Member

Would be good to upstream this patch.

@rfht
Copy link
Contributor Author

rfht commented Oct 26, 2023

PR to upstream here: #84017

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.

5 participants