Re: [PATCH v2 00/10] Avoid the comma operator

From: Jeff King <peff@peff.net>
Date: Mar 26, 2025, 17:50 UTC
Message-ID: <20250326175059.GB2508080@coredump.intra.peff.net>
In-Reply-To: <pull.1889.v2.git.1742945534.gitgitgadget@gmail.com>

On Tue, Mar 25, 2025 at 11:32:04PM +0000, Johannes Schindelin via GitGitGadget wrote:

Show 10 quoted lines

> The comma operator
> [https://en.cppreference.com/w/c/language/operator_other#Comma_operator] is
> rarely used in C anymore, and typically indicates a typo. Just like in these
> instances, where a semicolon was meant to be used, as there is no need to
> discard the first statement's result here.
> 
> Changes since v1:
> 
>  * Use -Wcomma when compiling with clang and with DEVELOPER=1.
>  * Address the remaining instances pointed out by clang (and by Phillip).

Thanks for fixing these. I checked the diff against the quick-and-dirty patch I posted earlier in the thread, and your resolutions for the "easy" cases look good (though like others, I'd prefer switching to semicolons for the one in diff-delta.c).

For the harder cases inside while() conditionals, the rewrites all look correct to me. I do think that getting rid of the commas often makes the result easier to read, but the discussion on wildmatch shows that it's easy to get the transformation wrong. So I'd be happy enough slapping a "(void)" on that one and moving on with our lives. The goal is not to prettify that code (which was not even written for Git in the first place) but to silence -Wcomma false positives so that we can find the actual problems.

-Peff