Re: [PATCH 09/13] parse-options API: don't restrict OPT_SUBCOMMAND() to one *_fn type

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Jeff King" <peff@peff.net>, "Taylor Blau" <me@ttaylorr.com>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Lukáš Doktor" <ldoktor@redhat.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 09/13] parse-options API: don't restrict OPT_SUBCOMMAND() to one *_fn type
Date: Sun, 06 Nov 2022 14:28:58 +0100	[thread overview]
Message-ID: <221106.86k048w2f4.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <46ee3f92-943f-10d5-7ec5-c68f1ced87ce@web.de>

On Sun, Nov 06 2022, René Scharfe wrote:

> Am 05.11.22 um 23:33 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Sat, Nov 05 2022, René Scharfe wrote:
>>
>>> Am 05.11.22 um 14:52 schrieb Ævar Arnfjörð Bjarmason:
>>>>
>>>> I think that's an "unportable" extension covered in "J.5 Common
>>>> extensions", specifically "J.5.7 Function pointer casts":
>>>>
>>>> 	A pointer to an object or to void may be cast to a pointer to a
>>>> 	function, allowing data to be invoked as a function
>>>>
>>>> Thus, since the standard already establishes that valid "void *" and
>>>> "intptr_t" pointers can be cast'd back & forth, the J.5.7 bridges the
>>>> gap between the two saying a function pointer can be converted to
>>>> either.
>>>>
>>>> Now, I may be missing something here, but I was under the impression
>>>> that "intptr_t" wasn't special in any way here, and that any casting of
>>>> a function pointer to either it or a "void *" was what was made portable
>>>> by "J.5.7".
>>>
>>> Do you mean "possible" or "workable" instead of "portable" here?  As you
>>> write above, J.5.7 is an extension, not (fully) portable.
>>
>> I think my just-sent in the side-thread should clarify this.
>
> AFAIU you think that J.5.7 plus POSIX make conversions between object
> pointers and function pointers portable.

No, I think that:

1) J.5.7 does that on its own for C99
2) POSIX has orthagonally mandated this, seperate from C99.

In practice I think it's always worked for dlsym(), but there's
interesting changes in wording between v6 and v7 of POSIX:

   - https://pubs.opengroup.org/onlinepubs/009695399/functions/dlsym.html
   - https://pubs.opengroup.org/onlinepubs/9699919799/functions/dlsym.html

v6 claims that conforming C compilers are required to produce a warning
if this isn't supported (I haven't found the part of the standard
they're referencing), and notes that the behavior may be deprecated in
the future.

Whereas v7 says that POSIX "requires this conversion to work correctly
on conforming implementations".

Is your reading of any of that different?

In addition to that: This is widely supported on systems that didn't
aquire such support via optional C99 extensions, or POSIX. E.g. Win32.

I think this is one of those things that C has left deliberately
undefined for the benefit of embedded implementations, and others with
odd memory models.

>> I think "just leave it, and see if anyone complains".
>>
>> If you look over config.mak.uname you can see what we're likely to be
>> ported to (and some of that's probably dead). The list of potential
>> targets that:
>>
>>  1) We know of ports to, or people would plausibly port git to
>>  2) Are updated so slow that they're on a release that's getting close
>>     to a year old.
>>
>> Are small, and it's usually easy to look up their memory model etc. are
>> you concerned about any specific one?
>
> Using implementation-defined behavior and requiring extensions when
> standard code would work just as well makes no sense to me.

I think it's useful in itself to see what subset or superset of C we
actually need to concern ourselves with.

E.g. we have plenty of code that assumes ASCII, instead of catering to
EBCDIC, and assuming NULL is (void *)0, not (void *)123456 or whatever.

Yes, in this case the alternative is trivial, but perhaps we'd find a
use-case in the future.

All I'm saying is let's leave the current one in place, as there's no
indication that it's not supported by our targets.