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

From: Junio C Hamano <[email-masked]>
Date: Fri, 31 Oct 2025 08:16:13 -0700

"Windl, Ulrich" <[email-masked]> writes:

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

>> -----Original Message-----
>> From: René Scharfe <[email-masked]>
>> Sent: Monday, October 6, 2025 7:24 PM
>> To: [email-masked]
>> Cc: Windl, Ulrich <[email-masked]>; Junio C Hamano <[email-masked]>;
>> Phillip Wood <[email-masked]>
>> 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