This repository has been archived by the owner on Feb 20, 2023. It is now read-only.
Toolchain related fixes #1192
Merged
Toolchain related fixes #1192
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
gcc-10 doesn't like how we didn't include cstdint in one file; in other cases, it didn't like the implicit cast to an unsigned. In another case, it didn't like that we return a uint64_t as an element of a uint32_t. In the last case, we now limit the amount of rows we can affect or return to 2^32. This is technically a functional change, but before, returning 4 billion rows would've caused issues at the network layer, so it isn't a *real* functional change.
Recasts to a void* before using memset in order to appease -Wclass-memaccess.
See b962fbb15c9f1fb2f5410ff0cf3a4065088c717e
See c61a9071ae9410daa629246c46a24165626f992b
clang-10 bugfix
Would like someone to review this to make sure I didn't hallucinate anything. Fixes clang-10 warnings.
gonzalezjo
added
the
ready-for-review
4 tasks
Codecov Report
@@ Coverage Diff @@
## master #1192 +/- ##
==========================================
- Coverage 82.77% 81.70% -1.07%
==========================================
Files 651 650 -1
Lines 46983 43674 -3309
==========================================
- Hits 38888 35683 -3205
+ Misses 8095 7991 -104
Continue to review full report at Codecov.
|
|
LGTM |
mbutrovich
added
best-practice
thepinetree
pushed a commit
to thepinetree/noisepage
that referenced
this pull request
Oct 13, 2020
* Fix gcc-10 uint32_t related issues gcc-10 doesn't like how we didn't include cstdint in one file; in other cases, it didn't like the implicit cast to an unsigned. In another case, it didn't like that we return a uint64_t as an element of a uint32_t. In the last case, we now limit the amount of rows we can affect or return to 2^32. This is technically a functional change, but before, returning 4 billion rows would've caused issues at the network layer, so it isn't a *real* functional change. * Fix gcc-10 -Wclass-memaccess issue Recasts to a void* before using memset in order to appease -Wclass-memaccess. * spdlog bugfix See b962fbb15c9f1fb2f5410ff0cf3a4065088c717e * json library bugfix See c61a9071ae9410daa629246c46a24165626f992b * Use references instead of copies clang-10 bugfix * Obscure numeric representability bugfix Would like someone to review this to make sure I didn't hallucinate anything. Fixes clang-10 warnings. * clang-format fixes oop
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This collection of fixes allows us to build using clang-10 and gcc-10.
Note that this does not allow us to build using llvm-10's libraries. This is purely addressing the new warnings. In the process, though, it does make some functional changes:
This just propagates the fundamental network limits through the type system.
(While I'm pretty sure a whole lot of other things will break before TrafficCop if we ever try to return 4 billion rows, this change had to be made in order for us to build under gcc-10)
These two functional changes are in need of review.