Xdiff cleanup part2
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v5 Latest10 patches 3 repliesIn review
- v410 patches 10 repliesSuperseded
- v310 patches 12 repliesSuperseded
Earlier versions · 2
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v2 | 10 patches | 12 replies | Superseded | |
| v1 | 9 patches | 27 replies | Superseded |
Changes in v5 from the cover letter
* Remove the non-word 'signless', and rephrase that paragraph in unambiguous-types.adoc
* Cast to char in xdiffi.c:get_indent() rather than changing the local variable to uint8_t
Patches in v5 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/10 | doc: define unambiguous type mappings across C and RustDocumentation/ Documentation/ Documentation/ | +226 | 2 replies | |
| 2/10 | xdiff: use ptrdiff_t for dstart/dendxdiff/ | +1 | 0 replies | |
| 3/10 | xdiff: make xrecord_t.ptr a uint8_t instead of charxdiff/ xdiff/ xdiff/ +4 more | +21 | 0 replies | |
| 4/10 | xdiff: use size_t for xrecord_t.sizexdiff/ xdiff/ xdiff/ +2 more | +19 | 0 replies | |
| 5/10 | xdiff: use unambiguous types in xdl_hash_record()xdiff-interface.c xdiff/ xdiff/ +1 more | +21 | 0 replies | |
| 6/10 | xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hashxdiff/ xdiff/ xdiff/ +2 more | +21 | 0 replies | |
| 7/10 | xdiff: make xdfile_t.nrec a size_t instead of longxdiff/ xdiff/ xdiff/ +3 more | +26 | 0 replies | |
| 8/10 | xdiff: make xdfile_t.nreff a size_t instead of longxdiff/ xdiff/ | +8 | 0 replies | |
| 9/10 | xdiff: change rindex from long to size_t in xdfile_txdiff/ | +1 | 0 replies | |
| 10/10 | xdiff: rename rindex -> reference_indexxdiff/ xdiff/ xdiff/ | +9 | 0 replies |
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v5 yet.
Cover letter [PATCH v5 00/10] Xdiff cleanup part2
Changes in v5:
* Remove the non-word 'signless', and rephrase that paragraph in unambiguous-types.adoc
* Cast to char in xdiffi.c:get_indent() rather than changing the local variable to uint8_t
Changes in v4:
* Update documentation to not mention Unicode except once
* Don't move dstart/dend with in the xdfile_t struct
* Rephrase justification on changing xrecord_t.ptr's type
Changes in v3:
* Address comments about commit messages and documentation
* Add unambiguous-types.adoc to Makefile and Meson
* Use markdown style to avoid asciidoc issues
Changes in v2:
* Added documentation about unambiguous types and FFI
* Addressed comments on the mailing list
Original cover letter below:
============================
Maintainer note: This patch series builds on top of en/xdiff-cleanup and am/xdiff-hash-tweak (both of which are now in master).
The primary goal of this patch series is to convert every field's type in xrecord_t and xdfile_t to be unambiguous, in preparation to make it more Rust FFI friendly. Additionally the ha field in xrecord_t is split into line_hash and minimal_perfect hash.
The order of some of the fields has changed as called out by the commit messages.
Before:
typedef struct s_xrecord {
char const *ptr;
long size;
unsigned long ha;
} xrecord_t;typedef struct s_xdfile {
xrecord_t *recs;
long nrec;
long dstart, dend;
bool *changed;
long *rindex;
long nreff;
} xdfile_t;After part 2
typedef struct s_xrecord {
uint8_t const *ptr;
size_t size;
uint64_t line_hash;
size_t minimal_perfect_hash;
} xrecord_t;typedef struct s_xdfile {
xrecord_t *recs;
size_t nrec;
ptrdiff_t dstart, dend;
bool *changed;
size_t *reference_index;
size_t nreff;
} xdfile_t;Ezekiel Newren (10):
doc: define unambiguous type mappings across C and Rust
xdiff: use ptrdiff_t for dstart/dend
xdiff: make xrecord_t.ptr a uint8_t instead of char
xdiff: use size_t for xrecord_t.size
xdiff: use unambiguous types in xdl_hash_record()
xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash
xdiff: make xdfile_t.nrec a size_t instead of long
xdiff: make xdfile_t.nreff a size_t instead of long
xdiff: change rindex from long to size_t in xdfile_t
xdiff: rename rindex -> reference_index
Documentation/Makefile | 1 + Documentation/technical/meson.build | 1 + .../technical/unambiguous-types.adoc | 224 ++++++++++++++++++ xdiff-interface.c | 2 +- xdiff/xdiffi.c | 29 ++- xdiff/xemit.c | 28 +-- xdiff/xhistogram.c | 4 +- xdiff/xmerge.c | 30 +-- xdiff/xpatience.c | 14 +- xdiff/xprepare.c | 60 ++--- xdiff/xtypes.h | 15 +- xdiff/xutils.c | 32 +-- xdiff/xutils.h | 6 +- 13 files changed, 336 insertions(+), 110 deletions(-) create mode 100644 Documentation/technical/unambiguous-types.adoc
base-commit: a99f379adf116d53eb11957af5bab5214915f91d
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2070%2Fezekielnewren%2Fxdiff_cleanup_part2-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2070/ezekielnewren/xdiff_cleanup_part2-v5
Pull-Request: https://github.com/git/git/pull/2070
Range-diff vs v4:
1: af732beb69 ! 1: 8b56bf1172 doc: define unambiguous type mappings across C and Rust
@@ Documentation/technical/unambiguous-types.adoc (new)
+has additional semantics and platform-dependent behavior that can cause +problems, as discussed below.
+
-+C comparison problem: While the sign of char is implementation defined, it's -+also signless (neither signed nor unsigned). When building with -+make DEVELOPER=1 it will complain about a "differ in signedness" when char -+is compared with uint8_t or int8_t.
++The C language leaves the signedness of char implementation defined. Because ++our developer build enables -Wsign-compare, comparison of a value of char ++type with either signed or unsigned integers may trigger warnings from the ++compiler.
+
+Note: Rust's char type is an unsigned 32-bit integer that is used to describe +Unicode code points.
2: b60a03eb31 = 2: c4193d11f5 xdiff: use ptrdiff_t for dstart/dend 3: 042fbb11d0 ! 3: dd76d4f586 xdiff: make xrecord_t.ptr a uint8_t instead of char
@@ Commit message
Make xrecord_t.ptr uint8_t because it's referring to bytes in memory.
In order to avoid a refactor avalanche, many uses of this field were
- cast to char* or similar. One exception is in get_indent() where the
- local variable char c was changed to uint8_t c.
+ cast to char* or similar.
Places where casting was unnecessary:
xemit.c:156
@@ xdiff/xdiffi.c: static int get_indent(xrecord_t *rec)
for (i = 0; i < rec->size; i++) {
- char c = rec->ptr[i];
-+ uint8_t c = rec->ptr[i];
++ char c = (char) rec->ptr[i];
if (!XDL_ISSPACE(c))
return ret;
4: c103fa6bea ! 4: 11cec1d2ec xdiff: use size_t for xrecord_t.size
@@ xdiff/xdiffi.c: static int recs_match(xrecord_t *rec1, xrecord_t *rec2)
- for (i = 0; i < rec->size; i++) {
+ for (size_t i = 0; i < rec->size; i++) {
- uint8_t c = rec->ptr[i];
+ char c = (char) rec->ptr[i];
if (!XDL_ISSPACE(c))
@@ xdiff/xdiffi.c: static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)
5: 2ee9a74653 = 5: 6f267360b7 xdiff: use unambiguous types in xdl_hash_record() 6: f044274bd5 = 6: 78af0f16f4 xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash 7: f7a3731d94 = 7: 5c19f9ded3 xdiff: make xdfile_t.nrec a size_t instead of long 8: 93f84ae72e = 8: d1f498edb1 xdiff: make xdfile_t.nreff a size_t instead of long 9: 39369becc8 = 9: bc4941c146 xdiff: change rindex from long to size_t in xdfile_t 10: 950d1e6193 = 10: dcc9d6bfaf xdiff: rename rindex -> reference_index
Signature · 1 line
gitgitgadget