Re: [PATCH 1/2] git-compat-util: st_add4: work around gcc 4.2.x compiler crash

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 21, 2016, 05:38 UTC
Message-ID: <CAPig+cR2rBJ4Tn9h7d4pp_Uxrfgc-BgHMLOhxKhWtYbm2wnARA@mail.gmail.com>
In-Reply-To: <CAP8UFD0Kp55bpwdNrXY3KhpDiHVDReWKpuNJz7_Q537DXeUXHA@mail.gmail.com>

On Mon, Mar 21, 2016 at 12:56 AM, Christian Couder <christian.couder@gmail.com> wrote:

Show 17 quoted lines

> On Mon, Mar 21, 2016 at 5:35 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
>>
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index c07e0c1..4743954 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -715,8 +715,8 @@ static inline size_t st_add(size_t a, size_t b)
>>                     (uintmax_t)a, (uintmax_t)b);
>>         return a + b;
>>  }
>> -#define st_add3(a,b,c)   st_add((a),st_add((b),(c)))
>> -#define st_add4(a,b,c,d) st_add((a),st_add3((b),(c),(d)))
>> +#define st_add3(a,b,c)   st_add(st_add((a),(b)),(c))
>> +#define st_add4(a,b,c,d) st_add(st_add3((a),(b),(c)),(d))
>
> Nit: maybe a comment around those lines would make sure that people do
> not inadvertently change them back later.

Maybe, maybe not. I'm hesitant for the following reason.

Unless we determine the exact compiler bug and patch which fixed it, we don't really have a good handle on what triggers this crash. Consequently, even with a comment saying "don't change the code in such and such a way", if someone ever does need to modify st_add4() in some fashion, it's entirely possible that the modification will trigger the crash again, even if the current evaluation order is kept or only modified slightly. We just don't know.

So, such a comment doesn't strike me as having a lot of value, and whatever value it does have wanes as this old compiler (gcc 4.2.1) and these old platforms (Mac OS X 10.6 and FreeBSD 9.x) become less and less relevant over time.