Re: [EXT] [PATCH v3 6/6] add-patch: reset "permitted" at loop start

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 31, 2025, 15:16 UTC
Message-ID: <xmqqfraz2jb6.fsf@gitster.g>
In-Reply-To: <77991a11c53f40b8b0a050a4d081809a@ukr.de>

"Windl, Ulrich" <u.windl@ukr.de> writes:

Show 5 quoted lines

> Just a comment of personal taste: I think declaring an anonymous
> enum inside a loop is just bad style. I think that gcc is smart
> enough to optimize if "permitted" is declared outside the loop, or
> make the "permitted" use a typedef for a "named enum" (declared
> outside the loop while the variable may be inside the loop).

If this is more than just a personal preference (which to me does sound like), a patch to improve it on top is very much welcomed.

The change itself would be just reverting the code movement, drop the 0 initialization and resetting the ariable at the top of the loop every iteration. But the rationale being that it would give compilers a chance to do a better job, I'd prefer to see a compiler person write the proposed log message, possibly backed by data (perhaps "generated assembly is objectively better---compare this and that" in this case? I dunno).

Thanks.

Show 24 quoted lines

>> -----Original Message-----
>> From: René Scharfe <l.s.r@web.de>
>> Sent: Monday, October 6, 2025 7:24 PM
>> To: git@vger.kernel.org
>> Cc: Windl, Ulrich <u.windl@ukr.de>; Junio C Hamano <gitster@pobox.com>;
>> Phillip Wood <phillip.wood@dunelm.org.uk>
>> Subject: [EXT] [PATCH v3 6/6] add-patch: reset "permitted" at loop start
>> 
> [...] 
>>  	for (;;) {
>> +		enum {
>> +			ALLOW_GOTO_PREVIOUS_HUNK = 1 << 0,
>> +			ALLOW_GOTO_PREVIOUS_UNDECIDED_HUNK = 1 <<
>> 1,
>> +			ALLOW_GOTO_NEXT_HUNK = 1 << 2,
>> +			ALLOW_GOTO_NEXT_UNDECIDED_HUNK = 1 << 3,
>> +			ALLOW_SEARCH_AND_GOTO = 1 << 4,
>> +			ALLOW_SPLIT = 1 << 5,
>> +			ALLOW_EDIT = 1 << 6
>> +		} permitted = 0;
>> +
>>  		if (hunk_index >= file_diff->hunk_nr)
>>  			hunk_index = 0;
>>  		hunk = file_diff->hunk_nr