Removing long (part 7): Various miscellaneous changes #31550
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZhilkinSerg
added
[C++]
|
MINGW and XCode are both unhappy: |
|
When building under MSYS2: |
item_location had a hacky workaround to suppress implicit conversion to int (give that it had a conversion to bool). This probably dates to an old compiler or something like that. Removed the workaround and simply made the conversion to bool explicit.
When overloading different integer types, it's necessary to explicitly use long, because otherwise you can't get exact coverage of the various integer types.
mission::value (which is what this function returns) had already been converted to int.
I'm not sure this function is even actually used, but in any case it needn't process longs.
SDL_GetTicks returns a Uint32, so this seems a good fit.
TTF_FontFaces returns a long, so it's a reasonable choice for this use case.
Lots of unnecessary longs here, like the number of tiles travelled.
This is only ever equal to 1 or 7, as far as I can see.
Amongst other things, switch mod_manager_ui selection from long to size_t This was written as if it might be negative, but none of the callers could ever make it so, and the code is just much simpler if it's a size_t.
jbytheway
force-pushed
the
remove_long_misc
branch
from
437d211 to
8bad9a9
Compare
ZhilkinSerg
reviewed
Jun 18, 2019
tests/map_memory.cpp
Outdated
| @@ -102,7 +102,8 @@ TEST_CASE( "lru_cache_perf", "[.]" ) | |||
| } | |||
| } | |||
| const auto end1 = std::chrono::high_resolution_clock::now(); | |||
| const long diff1 = std::chrono::duration_cast<std::chrono::microseconds>( end1 - start1 ).count(); | |||
| const int64_t diff1 = std::chrono::duration_cast<std::chrono::microseconds> | |||
| ( end1 - start1 ).count(); | |||
| printf( "completed %d insertions in %ld microseconds.\n", max_size, diff1 ); | |||
There was a problem hiding this comment.
Suggested change
| printf( "completed %d insertions in %ld microseconds.\n", max_size, diff1 ); | |
| printf( "completed %d insertions in %lld microseconds.\n", max_size, diff1 ); |
Can't find any format specifier that works portably for int64_t (PRId64 is problematic on MSYS) so just use long longs instead.
jbytheway
force-pushed
the
remove_long_misc
branch
from
8bad9a9 to
a58d755
Compare
|
Jenkins rebuild |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
SUMMARY: Infrastructure "Remove most uses of long and unsigned long from the code"
Purpose of change
My ongoing campaign to remove uses of
long.Describe the solution
Many assorted small changes. Mostly they just come down to changing
longorunsigned longto eitherint,size_t,uint32_t, orint64_tas seems appropriate, or suppressing the warnings iflongis the right choice. Some other things:boolonitem_location.See the individual commit comments for details.
Additional context
I think this is essentially all the remaining uses of
longcaught and dealt with, but there are probably a couple more lurking (there are some uses that my clang-tidy check can't find, for example in code that isn't compiled in this configuration).