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

Prefer fixed-width integer types over fundamental types like unsigned int. #2325

Closed
ChrisThrasher opened this issue Dec 31, 2022 · 8 comments
Labels
Projects
Milestone

Comments

@ChrisThrasher
Copy link
Member

Subject of the issue

Many interfaces and class members use unsigned int. This type was seemingly picked as a C++03 shorthand for std::size_t or at least an approximation for std::uint32_t even though unsigned int isn't necessarily 32 bits wide. If our intent is to use 32 bit (or 64 bit) unsigned integers, let's use a type which better expresses and guarantees our intent.

A similar argument could be used for any uses of short, int, or long although are not nearly as pervasive in the interface so I'm not necessarily calling those out.

See this cppreference page for more information on the various possible widths of integral types.

Screenshot 2022-12-30 at 9 38 18 PM

@ChrisThrasher ChrisThrasher added this to the 3.0 milestone Dec 31, 2022
@ChrisThrasher ChrisThrasher added this to Backlog in SFML 3.0.0 via automation Dec 31, 2022
@ChrisThrasher ChrisThrasher moved this from Backlog to Clean Up Changes in SFML 3.0.0 Dec 31, 2022
@ChrisThrasher ChrisThrasher moved this from Clean Up Changes to C++17 Changes in SFML 3.0.0 Dec 31, 2022
@ChrisThrasher
Copy link
Member Author

ChrisThrasher@fbe8fed

Starting off with unsigned short was easy so I went ahead and did that to help illustrate my point. Luckily unsigned short is always 16 bits so this one is unlikely if not impossible to have caused any bugs but I still think it's worth the added readability to know that we're working with 16 bit ints.

@Griezn
Copy link

Griezn commented Jun 4, 2023

I want to help with this. Is there any guideline in which cases to use which types? Let's say there is an unsigned long is it safe to assume it would likely be 32-bit?

@ChrisThrasher
Copy link
Member Author

We haven't yet made a decision about whether or not we want to do this so there is as of yet no work to do.

@ChrisThrasher
Copy link
Member Author

I've thought about this more and think that focusing on changing unsigned int to std::size_t in APIs is the most valuable change to make. C++ programmers are so used to seeing std::size_t as the type for quantities of things. SFML would benefit from following that pattern.

Beyond that I think the marginal value diminishes quickly so I probably won't push on this issue much farther.

@eXpl0it3r
Copy link
Member

Looking a bit around, it seems that if one doesn't have a reason for a specific fixed-width integer, it's more recommend to just stick with int. So I wouldn't really want to convert everything to a fixed-width integer, only if we can be specific as to why it should be 32-bits or 64-bits etc.

For indexes and "countable" things, I think it makes sense to prefer std::size_t over unsigned int.

@ChrisThrasher
Copy link
Member Author

ChrisThrasher commented Dec 5, 2023

https://www.stroustrup.com/JSF-AV-rules.pdf

Screenshot 2023-12-05 at 4 09 18 PM

The JSF Coding Guidelines written by C++'s inventor Bjarne Stroustrop recommended as far back as 2005 to avoid using variable-length integer types and provides some argument for doing so. This makes me feel even more confident that we don't need to keep using things like unsigned int going forward even when we're not dealing with counts of things. It's still not massively valuable to make this change but it would an improvement if we did.

@eXpl0it3r
Copy link
Member

I wouldn't put too much weight on just one guideline, especially if it was for a very specific industry (jet fighters) and predates C++11 by a lot, even if it's "from the C++'s inventor".

There are plenty of other guidelines out there that go in other directions. It's always easy to pick one example and argue why it's the "correct" source of truth. In the end our decision should be based on our needs/targets and not just because someone defined it for their objectives.

IMHO, as long as we don't have a reason for a fixed-width integer type, it's better to let the compiler pick the actual type and it can be argued would also be the "most efficient" type for that architecture.

@ChrisThrasher
Copy link
Member Author

Closing due to lack of enthusiasm from the team. I still think this issue has value but we need to triage for v3 and this isn't important enough to make the cut. Maybe in v4 we can reconsider our use of variable-width ints in our APIs.

@ChrisThrasher ChrisThrasher closed this as not planned Won't fix, can't repro, duplicate, stale Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
SFML 3.0.0
  
C++17 Changes
Development

No branches or pull requests

3 participants