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

When compiling to Windows (either on Linux or Windows) integer types (short/int/long) are incorrectly sized #1511

Open
MagicCraftMaster opened this issue Jun 27, 2024 · 3 comments

Comments

@MagicCraftMaster
Copy link

MagicCraftMaster commented Jun 27, 2024

Godot version

v4.2.2.stable.mono.official [15073afe3]

godot-cpp version

4.2.2

System information

Godot v4.2.2.stable.mono - Manjaro Linux ZEN SMP PREEMPT_DYNAMIC - X11 - Vulkan (Forward+) - dedicated AMD Radeon RX 6700 XT () - AMD Ryzen 7 3800X 8-Core Processor (16 Threads)

Issue description

When compiling a GDExtension to Windows from Linux - or on some Windows installs - types such as long are not considered the correct size.
Ex: using int64_t instead of long will compile but just using long won't compile for Windows builds.

Steps to reproduce

Create a simple GDExtension using long as a variable type in a function (either a return type or a parameter)
Attempt to compile to Windows via scons arch=x86_64 platform=windows target=template_debug
A large amount of errors related to method binding will occur.

Minimal reproduction project

Follow "Steps to reproduce"

@dsnopek
Copy link
Collaborator

dsnopek commented Jun 27, 2024

Using long as an argument on a bound method actually seems to work for me on Linux with GCC 11.4. I'm a little surprised, though! We don't provide a version of GetTypeInfo<T> for long, so I was expecting to see an error.

However, I tried on MSVC on Windows, and did manage to get an error along the lines I was expecting.

In any case, personally, I don't think we should support binding functions with arguments or return types of long or short or any of the related variants. C++ doesn't give a fixed size for those types, they are all defined as "having at least X bits" which means the compiler could chose to use a larger size:

Selection_175

See https://en.cppreference.com/w/cpp/language/types#Integral_types

I think that could eventually lead to some unexpected compatibility weirdness.

I personally think using types with a well defined size is better.

@MagicCraftMaster
Copy link
Author

MagicCraftMaster commented Jun 27, 2024

Would it not be ideal to alias the types to the fixed width versions?
Or maybe define them to something so they raise a much clearer error that says to use the fixed width versions?
Edit: Maybe also define the int types like Rust (i8, i16, i32, i64, u8, u16, u32, u64) for optional use?

@dsnopek
Copy link
Collaborator

dsnopek commented Jul 11, 2024

Or maybe define them to something so they raise a much clearer error that says to use the fixed width versions?

This would be nice, although, I'm not sure how to actually do it.

I'm not too crazy about the other ideas. I don't want to try to magically alias types or provide new types, when we've already got all the ones we need from standard C++. And, in either case, we'd need to make those changes in Godot first, since godot-cpp tries to mimick Godot's internal APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants