Build failure with -std=gnu23 (GCC 15 default)

Hi,

Upcoming GCC 15 defaults to -std=gnu23. git-2.47.0 fails to build as follows:

```
builtin/index-pack.c:97:8: error: expected ‘{’ before ‘thread_local’
   97 | struct thread_local {
      |        ^~~~~~~~~~~~
builtin/index-pack.c:120:15: error: expected ‘{’ before ‘thread_local’
  120 | static struct thread_local nothread_data;
      |               ^~~~~~~~~~~~
builtin/index-pack.c:151:15: error: expected ‘{’ before ‘thread_local’
  151 | static struct thread_local *thread_data;
      |               ^~~~~~~~~~~~
```

There may be more issues, but at the very least thread_local became a proper keyword in C23, so that will need renaming.

It should be possible to reproduce these with older GCC (and Clang) with -std=gnu23 or -std=c23 set manually.

thanks,
sam

Most of our code works fine in C23, but there are some new additions to the standard that conflict with either our struct tags or functions. With this series, the code compiles and passes the testsuite with -std=c23 on GCC 14.2.0.

brian m. carlson (2):
index-pack: rename struct thread_local
reflog: rename unreachable

 builtin/index-pack.c | 10 +++++-----
 reflog.c             |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

"thread_local" is a keyword in C23. To make sure that our code compiles on a wide variety of C versions, rename struct thread_local to "struct thread_local_data" to avoid a conflict.

---
 builtin/index-pack.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

index 9d23b41b3a..a6942233e5 100644
@@ -94,7 +94,7 @@ static LIST_HEAD(done_head);
 static size_t base_cache_used;
 static size_t base_cache_limit;
 
-struct thread_local {
+struct thread_local_data {
 	pthread_t thread;
 	int pack_fd;
 };

@@ -117,7 +117,7 @@ static struct object_entry *objects;
 static struct object_stat *obj_stat;
 static struct ofs_delta_entry *ofs_deltas;
 static struct ref_delta_entry *ref_deltas;
-static struct thread_local nothread_data;
+static struct thread_local_data nothread_data;
 static int nr_objects;
 static int nr_ofs_deltas;
 static int nr_ref_deltas;

@@ -148,7 +148,7 @@ static uint32_t input_crc32;
 static int input_fd, output_fd;
 static const char *curr_pack;
 
-static struct thread_local *thread_data;
+static struct thread_local_data *thread_data;
 static int nr_dispatched;
 static int threads_active;
 

@@ -390,7 +390,7 @@ static NORETURN void bad_object(off_t offset, const char *format, ...)
 	    (uintmax_t)offset, buf);
 }
 
-static inline struct thread_local *get_thread_data(void)
+static inline struct thread_local_data *get_thread_data(void)
 {
 	if (HAVE_THREADS) {
 		if (threads_active)

@@ -401,7 +401,7 @@ static inline struct thread_local *get_thread_data(void)
 	return &nothread_data;
 }
 
-static void set_thread_data(struct thread_local *data)
+static void set_thread_data(struct thread_local_data *data)
 {
 	if (threads_active)
 		pthread_setspecific(key, data);

In C23, "unreachable" is a macro that invokes undefined behavior if it is invoked. To make sure that our code compiles on a variety of C versions, rename unreachable to "is_unreachable".

---
 reflog.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 875ac1aa66..aeab78c9b7 100644
@@ -210,7 +210,7 @@ static void mark_reachable(struct expire_reflog_policy_cb *cb)
 	cb->mark_list = leftover;
 }
 
-static int unreachable(struct expire_reflog_policy_cb *cb, struct commit *commit, struct object_id *oid)
+static int is_unreachable(struct expire_reflog_policy_cb *cb, struct commit *commit, struct object_id *oid)
 {
 	/*
 	 * We may or may not have the commit yet - if not, look it

@@ -265,7 +265,7 @@ int should_expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 			return 1;
 		case UE_NORMAL:
 		case UE_HEAD:
-			if (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))
+			if (is_unreachable(cb, old_commit, ooid) || is_unreachable(cb, new_commit, noid))
 				return 1;
 			break;
 		}

Thank you for the fast response! It both works and the fixes look correct.

For the series:

Both of the patches look obviously good to me. I was a bit surprised that this is required in the first place as I thought we were passing -std=gnu99 to our compilers, but that is not the case with our current Makefile. So I must have been misremembering.

Thanks!

Patrick

We do for DEVELOPER=1 mode, but it wouldn't make sense to pass something unportable like "gnu99" for the general case.

-Peff

We can't do that because FreeBSD's system headers require C11 and won't work with -std=gnu99, in addition to the general unportability of that construct as mentioned by Peff. There's a comment to that effect in config.mak.dev.

I'm pleased you found the patches acceptable, though.

brian m. carlson (they/them or he/him)
Toronto, Ontario, CA