@matheusbsbs / matheus-bagliano / commits / ee798742bd
Fork 0

merge-recursive: silence -Wxor-used-as-pow warning

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>

Jeff King committed Jan 25, 2020 at 00:37 UTC ee798742bd35d88770e4ef05a7944b5783790e60
1 file changed +14 -5
merge-recursive.c
+14 -5
@@ -1712,6 +1712,15 @@ static char *find_path_for_conflict(struct merge_options *opt,
1712 return new_path;
1713 }
1714
1715 +/*
1716 + * Toggle the stage number between "ours" and "theirs" (2 and 3) by flipping
1717 + * the 1-bit.
1718 + */
1719 +static inline int flip_stage(int stage)
1720 +{
1721 + return stage ^ 1;
1722 +}
1723 +
1724 static int handle_rename_rename_1to2(struct merge_options *opt,
1725 struct rename_conflict_info *ci)
1726 {
@@ -1756,14 +1765,14 @@ static int handle_rename_rename_1to2(struct merge_options *opt,
1765 * such cases, we should keep the added file around,
1766 * resolving the conflict at that path in its favor.
1767 */
1759 - add = &ci->ren1->dst_entry->stages[2 ^ 1];
1768 + add = &ci->ren1->dst_entry->stages[flip_stage(2)];
1769 if (is_valid(add)) {
1770 if (update_file(opt, 0, add, a->path))
1771 return -1;
1772 }
1773 else
1774 remove_file_from_index(opt->repo->index, a->path);
1766 - add = &ci->ren2->dst_entry->stages[3 ^ 1];
1775 + add = &ci->ren2->dst_entry->stages[flip_stage(3)];
1776 if (is_valid(add)) {
1777 if (update_file(opt, 0, add, b->path))
1778 return -1;
@@ -1776,7 +1785,7 @@ static int handle_rename_rename_1to2(struct merge_options *opt,
1785 * rename/add collision. If not, we can write the file out
1786 * to the specified location.
1787 */
1779 - add = &ci->ren1->dst_entry->stages[2 ^ 1];
1788 + add = &ci->ren1->dst_entry->stages[flip_stage(2)];
1789 if (is_valid(add)) {
1790 add->path = mfi.blob.path = a->path;
1791 if (handle_file_collision(opt, a->path,
@@ -1797,7 +1806,7 @@ static int handle_rename_rename_1to2(struct merge_options *opt,
1806 return -1;
1807 }
1808
1800 - add = &ci->ren2->dst_entry->stages[3 ^ 1];
1809 + add = &ci->ren2->dst_entry->stages[flip_stage(3)];
1810 if (is_valid(add)) {
1811 add->path = mfi.blob.path = b->path;
1812 if (handle_file_collision(opt, b->path,
@@ -1846,7 +1855,7 @@ static int handle_rename_rename_2to1(struct merge_options *opt,
1855 path_side_1_desc = xstrfmt("version of %s from %s", path, a->path);
1856 path_side_2_desc = xstrfmt("version of %s from %s", path, b->path);
1857 ostage1 = ci->ren1->branch == opt->branch1 ? 3 : 2;
1849 - ostage2 = ostage1 ^ 1;
1858 + ostage2 = flip_stage(ostage1);
1859 ci->ren1->src_entry->stages[ostage1].path = a->path;
1860 ci->ren2->src_entry->stages[ostage2].path = b->path;
1861 if (merge_mode_and_contents(opt, a, c1,