git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "René Scharfe" <l.s.r@web.de>
Cc: 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: Thu, 14 Dec 2023 17:05:03 -0500	[thread overview]
Message-ID: <20231214220503.GA3320432@coredump.intra.peff.net> (raw)
In-Reply-To: <4d0b2a5f-305b-4350-b164-44923cb250d8@web.de>

On Thu, Dec 14, 2023 at 02:08:31PM +0100, René Scharfe wrote:

> > I don't even know that we'd need much of a weather-balloon patch. I
> > think it would be valid to do:
> >
> >   #ifndef bool
> >   #define bool int
> >
> > to handle pre-C99 compilers (if there even are any these days). Of
> > course we probably need some conditional magic to try to "#include
> > <stdbool.h>" for the actual C99. I guess we could assume C99 by default
> > and then add NO_STDBOOL as an escape hatch if anybody complains.
> 
> The semantics are slightly different in edge cases, so that fallback
> would not be fully watertight.  E.g. consider:
> 
>    bool b(bool cond) {return cond == true;}
>    bool b2(void) {return b(2);}

Yeah. b2() is wrong for passing "2" to a bool. I assumed that the
compiler would warn of that (at least for people on modern C99
compilers, not the fallback code), but it doesn't seem to. It's been a
long time since I've worked on a code base that made us of "bool", but I
guess that idea is that silently coercing a non-zero int to a bool is
reasonable in many cases (e.g., "bool found_foo = count_foos()").

I guess one could argue that b() is also sub-optimal, as it should just
say "return cond" or "return !cond" rather than explicitly comparing to
true/false. But I won't be surprised if it happens from time to time.

> A coding rule to not compare bools could mitigate that.  Or a rule to
> only use the values true and false in bool context and to only use
> logical operators on them.

That seems more complex than we want if our goal is just supporting
legacy systems that may or may not even exist. Given your example, I'd
be more inclined to just do a weather-balloon adding <stdbool.h> to
git-compat-util.h, and using "bool" in a single spot in the code. If
nobody screams after a few releases, we can consider it OK. If they do,
it's a trivial patch to convert back.

-Peff

  reply	other threads:[~2023-12-14 22:05 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 [this message]
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
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=20231214220503.GA3320432@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=doremylover123@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=l.s.r@web.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).