This repository was archived by the owner on May 12, 2021. It is now read-only.
Merged
Conversation
Prepare for the addition of CI32 -- a signed compressed integer.
Duplicate the functionality of xxxx_c64 in xxxx_cu64, in preparation for the addition of xxxx_ci64 (for signed integers) and the eventual removal of xxxx_64.
Change all invocations from NumUtil_xxxx_c32 to use the NumUtil_xxxx_cu32 and NumUtil_xxx_ci32 variants which are explicitly signed or unsigned.
Change IO methods invoked to use explicitly signed/unsigned versions rather than casting.
Change IO methods invoked to use explicitly signed/unsigned versions rather than casting.
All of these have the type of the lvalue show up in the diff, making it easy to confirm that the types match.
Adapt Read_C32/Write_C32/Read_C64/Write_C64 to Read_CU32/etc for variables representing memory sizes: String, Blob, Vector and Hash sizes in particular.
Change Read/Write C64 to CI64 for file positions, all of which are represented as signed 64-bit integers.
Doc IDs are signed. Delta doc IDs may signed or unsigned. Use the appropriate CI32 or CU32 read/write methods for both.
`freq` (num occurrences per document for a term) is generally represented as a uint32_t. Positions may be signed or unsigned -- it's not consistent. Just match up to existing usage.
Transition all remaining Read/Write C32/C64 to Read_CI64/etc.
|
|
||
| // Write doc_freq. | ||
| OutStream_Write_C32(outstream, doc_freq); | ||
| OutStream_Write_CU32(outstream, doc_freq); |
Contributor
There was a problem hiding this comment.
I think this should be OutStream_Write_CI32.
Contributor
|
👍 for doing this tedious work. (The build fails on Windows because Charmonizer doesn't support PRId32 yet.) |
Contributor
Author
|
I'll add a couple small commits before merging to address those issues. Thanks for the review! |
asfgit
pushed a commit
that referenced
this pull request
Apr 21, 2016
At present, InStream, OutStream, and NumberUtils provide an unsigned compressed format called C32/C64 -- and when we need signed representations, we just cast. To get rid of all these casts (and fix some sites missing casts), support signed and unsigned variants. This closes #42.
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
At present, InStream, OutStream, and NumberUtils provide an unsigned compressed format called C32/C64 -- and when we need signed representations, we just cast.
To get rid of all these casts (and fix some sites missing casts), support signed and unsigned variants.