Re: [PATCH v2 01/18] cleanup: rename variables that collide with Rust primitive type names
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Sep 17, 2025, 07:42 UTC
- Message-ID
- <CAPig+cQqKCbGpfp=ppmjKEOe+sDRu6BocDfenzqvQJHSMiKDHQ@mail.gmail.com>
- In-Reply-To
- <5f77f1bd5d986dc1f8d123919af24dd219e323e8.1758071798.git.gitgitgadget@gmail.com>
On Tue, Sep 16, 2025 at 9:17 PM Ezekiel Newren via GitGitGadget <gitgitgadget@gmail.com> wrote:
Show 6 quoted lines
> cleanup: rename variables that collide with Rust primitive type names > > Use a regex to find and rename variables that collide with Rust > primitive integer and float type names: > > git grep -n -E -e '\<([ui](8|16|32|64|size)|(f(32|64)))\>'
This explains what the patch is doing but doesn't explain why we want this change. Please update the commit message to describe the problem/issue the patch is trying to address, and then (if necessary) explain what the patch is doing.
Show 20 quoted lines
> Matches were reviewed and renamed. The remaining matches don't count
> because:
> - Rust source files:
> contrib/libgit-rs/src/config.rs
> contrib/libgit-sys/src/lib.rs
> t/t4018/rust-impl
> t/t4018/rust-trait
> - Intentional references:
> t/helper/test-parse-options.c (prints Rust int names)
> t/t0040-parse-options.sh (tests the above)
>
> View with --color-words to highlight the variable renames.
>
> Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
> ---
> diff --git a/odb.c b/odb.c
> @@ -913,7 +913,7 @@ void *odb_read_object_peeled(struct object_database *odb,
> {
> - unsigned long isize;
> + unsigned long isize_;Nit: It's minor, but I can't say I'm a fan of this approach to renaming variables. It would be better to come up with a name that is more meaningful if possible rather than merely appending an underscore. In this particular case, the original name, "isize", already fails to convey much meaning, but appears to have been named this way simply to avoid a collision with the existing function argument named "size". So, you could just as easily rename "isize" to "sz" or some such.
Same comment applies to the other renamed variables...
> - OPT_UNSIGNED(0, "u16", &u16, "get a 16 bit unsigned integer"), > + OPT_UNSIGNED(0, "u16", &u16_, "get a 16 bit unsigned integer"),
... though with some of them, such as this one, it is admittedly more difficult to come up with a better name since the original name is already meaningful.