]> git.ipfire.org Git - thirdparty/git.git/commitdiff
merge-recursive: silence -Wxor-used-as-pow warning
authorJeff King <peff@peff.net>
Sat, 25 Jan 2020 05:37:23 +0000 (00:37 -0500)
committerJunio C Hamano <gitster@pobox.com>
Mon, 27 Jan 2020 19:15:35 +0000 (11:15 -0800)
The merge-recursive code uses stage number constants like this:

  add = &ci->ren1->dst_entry->stages[2 ^ 1];
  ...
  add = &ci->ren2->dst_entry->stages[3 ^ 1];

The xor has the effect of flipping the "1" bit, so that "2 ^ 1" becomes
"3" and "3 ^ 1" becomes "2", which correspond to the "ours" and "theirs"
stages respectively.

Unfortunately, clang-10 and up issue a warning for this code:

  merge-recursive.c:1759:40: error: result of '2 ^ 1' is 3; did you mean '1 << 1' (2)? [-Werror,-Wxor-used-as-pow]
                  add = &ci->ren1->dst_entry->stages[2 ^ 1];
                                                     ~~^~~
                                                     1 << 1
  merge-recursive.c:1759:40: note: replace expression with '0x2 ^ 1' to silence this warning

We could silence it by using 0x2, as the compiler mentions. Or by just
using the constants "2" and "3" directly. But after digging into it, I
do think this bit-flip is telling us something. If we just wrote:

  add = &ci->ren2->dst_entry->stages[2];

for the second one, you might think that "ren2" and "2" correspond. But
they don't. The logic is: ren2 is theirs, which is stage 3, but we
are interested in the opposite side's stage, so flip it to 2.

So let's keep the bit-flipping, but let's also put it behind a named
function, which will make its purpose a bit clearer. This also has the
side effect of suppressing the warning (and an optimizing compiler
should be able to easily turn it into a constant as before).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge-recursive.c

index 10dca5644b7b60f899cf1ef64e01550505d1fd8d..e6aedd3cab7427e13986fa102a4c56b092b73182 100644 (file)
@@ -1712,6 +1712,15 @@ static char *find_path_for_conflict(struct merge_options *opt,
        return new_path;
 }
 
+/*
+ * Toggle the stage number between "ours" and "theirs" (2 and 3) by flipping
+ * the 1-bit.
+ */
+static inline int flip_stage(int stage)
+{
+       return stage ^ 1;
+}
+
 static int handle_rename_rename_1to2(struct merge_options *opt,
                                     struct rename_conflict_info *ci)
 {
@@ -1756,14 +1765,14 @@ static int handle_rename_rename_1to2(struct merge_options *opt,
                 * such cases, we should keep the added file around,
                 * resolving the conflict at that path in its favor.
                 */
-               add = &ci->ren1->dst_entry->stages[2 ^ 1];
+               add = &ci->ren1->dst_entry->stages[flip_stage(2)];
                if (is_valid(add)) {
                        if (update_file(opt, 0, add, a->path))
                                return -1;
                }
                else
                        remove_file_from_index(opt->repo->index, a->path);
-               add = &ci->ren2->dst_entry->stages[3 ^ 1];
+               add = &ci->ren2->dst_entry->stages[flip_stage(3)];
                if (is_valid(add)) {
                        if (update_file(opt, 0, add, b->path))
                                return -1;
@@ -1776,7 +1785,7 @@ static int handle_rename_rename_1to2(struct merge_options *opt,
                 * rename/add collision.  If not, we can write the file out
                 * to the specified location.
                 */
-               add = &ci->ren1->dst_entry->stages[2 ^ 1];
+               add = &ci->ren1->dst_entry->stages[flip_stage(2)];
                if (is_valid(add)) {
                        add->path = mfi.blob.path = a->path;
                        if (handle_file_collision(opt, a->path,
@@ -1797,7 +1806,7 @@ static int handle_rename_rename_1to2(struct merge_options *opt,
                                return -1;
                }
 
-               add = &ci->ren2->dst_entry->stages[3 ^ 1];
+               add = &ci->ren2->dst_entry->stages[flip_stage(3)];
                if (is_valid(add)) {
                        add->path = mfi.blob.path = b->path;
                        if (handle_file_collision(opt, b->path,
@@ -1846,7 +1855,7 @@ static int handle_rename_rename_2to1(struct merge_options *opt,
        path_side_1_desc = xstrfmt("version of %s from %s", path, a->path);
        path_side_2_desc = xstrfmt("version of %s from %s", path, b->path);
        ostage1 = ci->ren1->branch == opt->branch1 ? 3 : 2;
-       ostage2 = ostage1 ^ 1;
+       ostage2 = flip_stage(ostage1);
        ci->ren1->src_entry->stages[ostage1].path = a->path;
        ci->ren2->src_entry->stages[ostage2].path = b->path;
        if (merge_mode_and_contents(opt, a, c1,