All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Jeff King" <peff@peff.net>, "René Scharfe" <l.s.r@web.de>,
	"AtariDreams via GitGitGadget" <gitgitgadget@gmail.com>,
	git@vger.kernel.org, "Seija Kijin" <doremylover123@gmail.com>
Subject: Re: [PATCH] Use ^=1 to toggle between 0 and 1
Date: Mon, 18 Dec 2023 16:18:36 +0000	[thread overview]
Message-ID: <ff4a6abc-8abf-4dbf-a787-e4895a78b048@gmail.com> (raw)
In-Reply-To: <xmqqo7erl7er.fsf@gitster.g>

Hi Junio

On 15/12/2023 17:09, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> Even if it unlikely that we would directly compare a boolean variable
>> to "true" or "false" it is certainly conceivable that we'd compare two
>> boolean variables directly. For the integer fallback to be safe we'd
>> need to write
>>
>> 	if (!cond_a == !cond_b)
>>
>> rather than
>>
>> 	if (cond_a == cond_b)
> 
> Eek, it defeats the benefit of using true Boolean type if we had to
> train ourselves to write the former, doesn't it?

Yes, it's horrible - if for some reason it turns out that we cannot use 
"#include <stdbool.h>" everywhere I think we should drop it rather than 
providing a subtly incompatible fallback

>> A weather-balloon seems like the safest route forward. We have been
>> requiring C99 for two years now [1], hopefully there aren't any
>> compilers out that claim to support C99 but don't provide
>> "<stdbool.h>" (I did check online and the compiler on NonStop does
>> support _Bool).
>>
>> Best Wishes
>>
>> Phillip
>>
>> [1] 7bc341e21b (git-compat-util: add a test balloon for C99 support,
>> 2021-12-01)
> 
> Nice to be reminded of this one.
> 
> The cited commit does not start to use any specific feature from
> C99, other than that we now require that the compiler claims C99
> conformance by __STDC_VERSION__ set appropriately.  The commit log
> message says C99 "provides a variety of useful features, including
> ..., many of which we already use.", which implies that our wish was
> to officially allow any and all features in C99 to be used in our
> codebase after a successful flight of this test balloon.
> 
> Now, I think we saw a successful flight of this test balloon by now.
> Is allowing all the C99 the next step we really want to take?
 >
> I still personally have an aversion against decl-after-statement and
> //-comments, not due to portability reasons at all, but because I
> find that the code is easier to read without it. But in principle,
> it is powerful to be able to say "OK, as long as the feature is in
> C99 you can use it", instead of having to decide on individual
> features, and I am not fundamentally against going that route if it
> is where people want to go.

I'm not sure we necessarily want to say "use anything that is in C99" 
for several reasons.

  - Some features such as C99's variable length arrays are known to be
    problematic.

  - As you say above there maybe features that we think harm the
    readability of our code.

  - As René points out not all compilers necessarily support all
    features.

I think using _Bool could be useful for the reasons Peff outlined. As 
for other features I've written code that I think would have benefited 
from compound literals, but off the top of my head I can't think of any 
other C99 features that I personally wish we were using. I think that 
decl-after-statement is occasionally useful to declare a variable near 
where it is used in a long function body but it is much simpler just to 
ban it altogether and encourage people to break up long functions to 
make them more readable.

Best Wishes

Phillip

> Thanks.
> 
> 

  parent reply	other threads:[~2023-12-18 16:18 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-12 17:17 [PATCH] Use ^=1 to toggle between 0 and 1 AtariDreams via GitGitGadget
2023-12-12 17:29 ` Dragan Simic
2023-12-12 20:09 ` Jeff King
2023-12-12 22:30   ` René Scharfe
2023-12-13  8:01     ` Jeff King
2023-12-13 15:17       ` Junio C Hamano
2023-12-14 13:08       ` René Scharfe
2023-12-14 22:05         ` Jeff King
2023-12-15 14:46           ` Phillip Wood
2023-12-15 17:09             ` Junio C Hamano
2023-12-16 10:46               ` René Scharfe
2023-12-18 16:18               ` Phillip Wood [this message]
2023-12-16 10:47             ` [PATCH] git-compat-util: convert skip_{prefix,suffix}{,_mem} to bool René Scharfe
2023-12-18 16:23               ` Phillip Wood
2023-12-18 20:19                 ` Junio C Hamano
2023-12-19 13:36                   ` René Scharfe
2023-12-21  9:59               ` Jeff King
2023-12-21  9:56             ` [PATCH] Use ^=1 to toggle between 0 and 1 Jeff King
2023-12-21 15:06               ` phillip.wood123
2024-12-18  0:16 ` [PATCH v2] " AreaZR via GitGitGadget
2024-12-18  0:42   ` [PATCH v3] git: use " AreaZR via GitGitGadget
2024-12-18  2:38     ` [PATCH v4] " AreaZR via GitGitGadget
2024-12-18 16:46       ` [PATCH v5] " AreaZR via GitGitGadget
2024-12-18 16:57         ` [PATCH v6] git: use logical-not operator " AreaZR via GitGitGadget
2024-12-19 10:35           ` Junio C Hamano
2024-12-18 15:46     ` [PATCH v3] git: use ^=1 " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ff4a6abc-8abf-4dbf-a787-e4895a78b048@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=doremylover123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.