ci: build with ENABLE_WERROR on Windows #5143
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
Build with -Werror's equivalent (/WX) on MSVC
ethomson
force-pushed
the
ethomson/warnings
branch
2 times, most recently
from
cf889f7
to
c934c97
Compare
ethomson
force-pushed
the
ethomson/warnings
branch
from
c934c97
to
b6b2d9d
Compare
Merged
When reallocating commit arrays in `opts_add_commit` and `opts_add_refish`, respectively, we simply pass the const pointer to `xrealloc`. As `xrealloc` expects a non-const pointer, though, this will generate a warning with some compilers. Cast away the constness to silence compilers.
The memchr(3P) function expects a `size_t` as its last parameter, but we do pass it an object size, which is of signed type `git_off_t`. As we can be sure that the result will be non-negative, let's just cast the parameter to a `size_t`.
When computing the progress, we perform some arithmetics that are implicitly converting from `size_t` to `int`. In one case we're calclulating a percentage, so we know that it should always be in the range of [0,100] and thus we're fine. In the other case we convert from bytes to kilobytes -- this should be stored in a `size_t` to avoid loss of precision, even though it probably won't matter due to limited download rates.
When iterating over index entries, we store the indices in an unsigned int. As the index entrycount is a `size_t` though, this may be a loss of precision which a compiler might rightfully complain about. Use `size_t` instead to fix any warnings.
We use the `__LINE__` macro in several places throughout clar to allow easier traceability when e.g. a test fails. While `__LINE__` is of type `size_t`, the clar functions all accept an integer and thus may loose precision. While unlikely that any file in our codebase will exceed a linecount of `INT_MAX`, let's convert it anyway to silence any compiler warnings.
In both `git_win32__stack_format` and `git_win32__stack`, we handle buffer lengths via an integer variable. As we only ever pass buffer sizes to it, this should be a `size_t` though to avoid loss of precision. As we also use it to compare with other `size_t` variables, this also silences signed/unsigned comparison warnings.
The function `git_win32__stack__set_aux_cb` expects the second parameter to be a function callback of type `git_win32__stack__aux_cb_lookup`, which expects a `size_t` parameter. In our test suite trace::windows::stacktrace, we declare the callback with `unsigned int` as parameter, though, causing a compiler warning. Correct the parameter type to silence the warning.
pks-t
approved these changes
Jul 5, 2019
|
Hope you don't mind me pushing this forward! :) |
|
ON THE CONTRARY. 🎉🎉🎉🎉🎉 |
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.
Build with -Werror's equivalent (/WX) on MSVC.
I neglected to do this in the previous warnings PR, oops. I think that there are also some lingering issues in things like examples that I didn't build locally with
/WXand missed since I didn't add this in the CI setup.