This is an archive of the discontinued Mercurial Phabricator instance.

dirstate: make dirstate flags char be unsigned
ClosedPublic

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

Download Raw Diff

Details

Reviewers

Alphare
marmoute

Group Reviewers

hg-reviewers

Commits

rHGec178161a8d1: dirstate: make dirstate flags char be unsigned

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

Herald added a reviewer: hg-reviewers. · View Herald TranscriptSep 16 2021, 8:06 PM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

martinvonz added a child revision: D11445: dirstate: fix compilation warnings in `dirstate_item_set_possibly_dirty()`.Sep 16 2021, 8:06 PM

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.

In D11444#175525, @Alphare wrote:

@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.

Done!

martinvonz added a commit: rHGec178161a8d1: dirstate: make dirstate flags char be unsigned.Sep 17 2021, 11:15 AM

Closed by commit rHGec178161a8d1: dirstate: make dirstate flags char be unsigned (authored by martinvonz). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/cext/parsers.c (7 lines)
M			mercurial/cext/util.h (18 lines)

	Status	Author	Revision
	Closed	martinvonz	D11445 dirstate: fix compilation warnings in `dirstate_item_set_possibly_dirty()`
	Closed	martinvonz	D11444 dirstate: make dirstate flags char be unsigned

Diff 30289

mercurial/cext/parsers.c


	static inline bool dirstate_item_c_tracked(dirstateItemObject *self)			static inline bool dirstate_item_c_tracked(dirstateItemObject *self)
	{			{
	return (self->flags & dirstate_flag_wc_tracked);			return (self->flags & dirstate_flag_wc_tracked);
	}			}

	static inline bool dirstate_item_c_added(dirstateItemObject *self)			static inline bool dirstate_item_c_added(dirstateItemObject *self)
	{			{
	char mask = (dirstate_flag_wc_tracked \| dirstate_flag_p1_tracked \|			unsigned char mask =
				(dirstate_flag_wc_tracked \| dirstate_flag_p1_tracked \|
	dirstate_flag_p2_tracked);			dirstate_flag_p2_tracked);
	char target = dirstate_flag_wc_tracked;			unsigned char target = dirstate_flag_wc_tracked;
	return (self->flags & mask) == target;			return (self->flags & mask) == target;
	}			}

	static inline bool dirstate_item_c_removed(dirstateItemObject *self)			static inline bool dirstate_item_c_removed(dirstateItemObject *self)
	{			{
	if (self->flags & dirstate_flag_wc_tracked) {			if (self->flags & dirstate_flag_wc_tracked) {
	return false;			return false;
	}			}

mercurial/cext/util.h

	#define PY23(py2, py3) py3			#define PY23(py2, py3) py3
	#else			#else
	#define PY23(py2, py3) py2			#define PY23(py2, py3) py2
	#endif			#endif

	/* clang-format off */			/* clang-format off */
	typedef struct {			typedef struct {
	PyObject_HEAD			PyObject_HEAD
	char flags;			unsigned char flags;
	int mode;			int mode;
	int size;			int size;
	int mtime;			int mtime;
	} dirstateItemObject;			} dirstateItemObject;
	/* clang-format on */			/* clang-format on */

	static const char dirstate_flag_wc_tracked = 1;			static const unsigned char dirstate_flag_wc_tracked = 1;
	static const char dirstate_flag_p1_tracked = 1 << 1;			static const unsigned char dirstate_flag_p1_tracked = 1 << 1;
	static const char dirstate_flag_p2_tracked = 1 << 2;			static const unsigned char dirstate_flag_p2_tracked = 1 << 2;
	static const char dirstate_flag_possibly_dirty = 1 << 3;			static const unsigned char dirstate_flag_possibly_dirty = 1 << 3;
	static const char dirstate_flag_merged = 1 << 4;			static const unsigned char dirstate_flag_merged = 1 << 4;
	static const char dirstate_flag_clean_p1 = 1 << 5;			static const unsigned char dirstate_flag_clean_p1 = 1 << 5;
	static const char dirstate_flag_clean_p2 = 1 << 6;			static const unsigned char dirstate_flag_clean_p2 = 1 << 6;
	static const char dirstate_flag_rust_special = 1 << 7;			static const unsigned char dirstate_flag_rust_special = 1 << 7;

	extern PyTypeObject dirstateItemType;			extern PyTypeObject dirstateItemType;
	#define dirstate_tuple_check(op) (Py_TYPE(op) == &dirstateItemType)			#define dirstate_tuple_check(op) (Py_TYPE(op) == &dirstateItemType)

	#ifndef MIN			#ifndef MIN
	#define MIN(a, b) (((a) < (b)) ? (a) : (b))			#define MIN(a, b) (((a) < (b)) ? (a) : (b))
	#endif			#endif
	/* VC9 doesn't include bool and lacks stdbool.h based on my searching */			/* VC9 doesn't include bool and lacks stdbool.h based on my searching */

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	30282		Sep 16 2021, 8:06 PM	★	★
Diff 2	30289	rHGec178161a8d123e1ffd555a5e2811f7deb223a76	Sep 16 2021, 7:29 PM	★	★