Page MenuHomePhabricator

dirstate: make dirstate flags char be unsigned
ClosedPublic

Authored by martinvonz on Sep 16 2021, 8:06 PM.

Details

Summary

Since https://phab.mercurial-scm.org/D11387, `CC='clang -Werror' make
local` has started failing like this:

mercurial/cext/util.h:41:50: error: implicit conversion from 'int' to 'char' changes value from 128 to -128 [-Werror,-Wconstant-conversion]
static const char dirstate_flag_rust_special = 1 << 7;
                  ~~~~~~~~~~~~~~~~~~~~~~~~~~   ~~^~~~

This patch fixes that by making the flags be an unsigned char. That
also matches the bool typedef we have in util.h, which seems good
since many of the dirstate_item_c_*() functions return a bool.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Sep 16 2021, 8:06 PM
Alphare accepted this revision.Sep 17 2021, 4:38 AM
Alphare added a subscriber: Alphare.

Thanks for catching this. We'll have to adjust the Heptapod CI to pick up on these as well, I'm surprised it does not.

This revision is now accepted and ready to land.Sep 17 2021, 4:38 AM
marmoute accepted this revision.Sep 17 2021, 8:39 AM

@martinvonz clangformat complains:

@@ -8,3 +8,18 @@
   >   cmp $f $f.formatted || diff -u $f $f.formatted
   >   rm $f.formatted
   > done
+  mercurial/cext/parsers.c mercurial/cext/parsers.c.formatted differ: char 3857, line 147
+  --- mercurial/cext/parsers.c        2021-09-17 15:05:49.010806214 +0000
+  +++ mercurial/cext/parsers.c.formatted      2021-09-17 15:06:34.139000016 +0000
+  @@ -144,8 +144,9 @@
+
+   static inline bool dirstate_item_c_added(dirstateItemObject *self)
+   {
+  -   unsigned char mask = (dirstate_flag_wc_tracked | dirstate_flag_p1_tracked |
+  -                dirstate_flag_p2_tracked);
+  +   unsigned char mask =
+  +       (dirstate_flag_wc_tracked | dirstate_flag_p1_tracked |
+  +        dirstate_flag_p2_tracked);
+      unsigned char target = dirstate_flag_wc_tracked;
+      return (self->flags & mask) == target;
+   }

I can just amend this if you want.

@martinvonz clangformat complains:

@@ -8,3 +8,18 @@
   >   cmp $f $f.formatted || diff -u $f $f.formatted
   >   rm $f.formatted
   > done
+  mercurial/cext/parsers.c mercurial/cext/parsers.c.formatted differ: char 3857, line 147
+  --- mercurial/cext/parsers.c        2021-09-17 15:05:49.010806214 +0000
+  +++ mercurial/cext/parsers.c.formatted      2021-09-17 15:06:34.139000016 +0000
+  @@ -144,8 +144,9 @@
+
+   static inline bool dirstate_item_c_added(dirstateItemObject *self)
+   {
+  -   unsigned char mask = (dirstate_flag_wc_tracked | dirstate_flag_p1_tracked |
+  -                dirstate_flag_p2_tracked);
+  +   unsigned char mask =
+  +       (dirstate_flag_wc_tracked | dirstate_flag_p1_tracked |
+  +        dirstate_flag_p2_tracked);
+      unsigned char target = dirstate_flag_wc_tracked;
+      return (self->flags & mask) == target;
+   }

I can just amend this if you want.

Oh, right, I realized late last night that I had forgotten to run the formatter, then I forgot about *that* over night :P Yes, please amend. Thanks.

This revision was automatically updated to reflect the committed changes.