Subject: [PATCH v5 00/10] Xdiff cleanup part2

From: "Ezekiel Newren via GitGitGadget" <[email-masked]>
Date: Tue, 18 Nov 2025 22:34:12 +0000

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

-- 
gitgitgadget