Message ID | pull.1889.v2.git.1742945534.gitgitgadget@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | Avoid the comma operator | expand |
On Tue, Mar 25, 2025 at 11:32:04PM +0000, Johannes Schindelin via GitGitGadget wrote: > 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 all of these fixes! Patrick
On Tue, Mar 25, 2025 at 11:32:04PM +0000, Johannes Schindelin via GitGitGadget wrote: > 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
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). Johannes Schindelin (10): remote-curl: avoid using the comma operator unnecessarily rebase: avoid using the comma operator unnecessarily kwset: avoid using the comma operator unnecessarily clar: avoid using the comma operator unnecessarily xdiff: avoid using the comma operator unnecessarily diff-delta: explicitly mark intentional use of the comma operator wildmatch: explicitly mark intentional use of the comma operator compat/regex: explicitly mark intentional use of the comma operator clang: warn when the comma operator is used detect-compiler: detect clang even if it found CUDA builtin/rebase.c | 2 +- compat/regex/regex_internal.c | 7 +++-- compat/regex/regexec.c | 2 +- config.mak.dev | 4 +++ detect-compiler | 2 +- diff-delta.c | 12 ++++---- kwset.c | 54 +++++++++++++++++++---------------- remote-curl.c | 4 +-- t/unit-tests/clar/clar/fs.h | 10 +++++-- wildmatch.c | 2 +- xdiff/xdiffi.c | 12 +++++--- 11 files changed, 65 insertions(+), 46 deletions(-) base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1889%2Fdscho%2Fcomma-operator-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1889/dscho/comma-operator-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1889 Range-diff vs v1: 1: e3069fd4564 ! 1: 913c7a0d296 remote-curl: avoid using the comma operator unnecessarily @@ Commit message Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> ## remote-curl.c ## +@@ remote-curl.c: static int fetch_git(struct discovery *heads, + packet_buf_flush(&preamble); + + memset(&rpc, 0, sizeof(rpc)); +- rpc.service_name = "git-upload-pack", ++ rpc.service_name = "git-upload-pack"; + rpc.gzip_request = 1; + + err = rpc_service(&rpc, heads, args.v, &preamble, &rpc_result); @@ remote-curl.c: static int push_git(struct discovery *heads, int nr_spec, const char **specs) packet_buf_flush(&preamble); 2: 7dfbdc48954 = 2: 37ff88b8275 rebase: avoid using the comma operator unnecessarily -: ----------- > 3: f601f4e74a5 kwset: avoid using the comma operator unnecessarily -: ----------- > 4: f60ebe376e1 clar: avoid using the comma operator unnecessarily -: ----------- > 5: 7239078413f xdiff: avoid using the comma operator unnecessarily -: ----------- > 6: 5e0e8325620 diff-delta: explicitly mark intentional use of the comma operator -: ----------- > 7: 9a6de12b807 wildmatch: explicitly mark intentional use of the comma operator -: ----------- > 8: dc626f36df3 compat/regex: explicitly mark intentional use of the comma operator -: ----------- > 9: 91f86c3aba9 clang: warn when the comma operator is used -: ----------- > 10: 2f6f31240fe detect-compiler: detect clang even if it found CUDA