Re: [PATCH v1 1/2] [troff]: Add lengthof() macro.

[Alejandro Colomar]

Hi Branden!

On 2023-08-26 00:28, G. Branden Robinson wrote:
> Hi Alex,
> 
> I didn't forget about this.
> 
> At 2023-08-04T03:00:10+0200, Alejandro Colomar wrote:
>> *  src/roff/troff/env.cpp (lengthof): Add macro to calculate the number
>>    of elements in an array.  It's named after the proposal to ISO C,
>>    _Lengthof(), which wasn't accepted for C23, but hopefully will be
>>    added in a future revision.
>>
>> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> 
> I have good news and bad news.

I have good news and bad news.  ;)

> 
> The bad news is that I'm rejecting this patch.

> 
> The good news is that the idea is good, and I've landed a form of it in
> Git.
> 
> https://git.savannah.gnu.org/cgit/groff.git/commit/?id=6d5987c97b63589328daff049c7b0f7220bb4149

The bad news is I'm rejecting your patch.

The good news is that I like the implementation.  I just don't like the
name.  I have a sizeof_array() macro that does

#define sizeof_array(a)  (sizeof(a) + must_be_array(a))

That is, it calculates the size in bytes that the array takes up in memory.

<https://github.com/shadow-maint/shadow/pull/762/commits/8d06d849dcb5f7041e048d866ec7ce6c1853245b>

For a macro that returns the number of elements in an array, I'd like a
name that cannot be confused with that at all.  NELEMS(), NITEMS(),
array_count(), or lengthof() all seem better than array_size().

I'm not a fan of lengthof(), even if it's a proposal to ISO C, as so far
the term "length" was only the number of non-zero characters in a string,
and overloading it to mean the number of elements in an array would
similarly be a bad thing.  At least it's not so confusing as size, though.

NITEMS() or NELEMS() seems the best choice to me.

I had a similar complaint to this one to kernel people, which were even
more evil than you, and having a macro ARRAY_SIZE() that returns the
number of elements, later added a function array_size() that returns the
size of a (dynamic) array in bytes.  I'd like you to avoid being evil.

<https://lore.kernel.org/lkml/57a30a95-b63c-7ad4-070f-db70262b6b7c@kernel.org/>

I recently remembered I had a similar discussion with some GCC (and WG14)
members some time ago, suggesting that the _Lengthof() proposal be
modified to be _Nitems().

<https://lore.kernel.org/linux-man/7697ff60-aa2a-a41e-9d08-ab25423ee750@gmail.com/T/#mbb7726dcb8a9f0ca3b4ac5eefb98f95f8d670773>

> 
>> I added it there because I didn't find a "common utilities" header
>> file.  Please suggest a better place.
> 
> I put it in src/include/lib.h.

Ok.  I'll consider it for similarly generic stuff.

> 
> At 2023-08-04T15:40:30+0200, Alejandro Colomar wrote:
>>> As I understand it, it is not idiomatic C++ to rely on the
>>> preprocessor any more than is necessary--and that means, again AIUI,
>>> to interpolate the text of header files and interface with C code.
>>
>> In C++17, I'd just call std::size().
>>
>> In C11 (or C++11), I'd add a static_assert(3) to that macro to make it
>> safer (but compiler warnings already make it reasonably safe[1]).
>>
>> In a mix of C++98 / C99, there's nothing I know of.  We could use
>> templates, which is how I bet std::size() is implemented, but I don't
>> have enough experience with them to do this kind of magic.
> 
> I added a template function.  I had intended to do a giant migration of
> everything that used the sizeof division trick, but that ran into
> problems.  One is that you can't apply a template to an anonymous type
> (at least in an obvious way).  That's solvable by giving names to
> structs.  But that ran into access problems with C++'s "friend"
> visibility rules, which I don't understand.
> 
> So I now think an opportunistic migration is a better approach than a
> flag day while I learn more about the language.

Sounds reasonable.

> 
>> I suggest a first implementation using a macro, which we know to work,
>> and then I'll let you improve on that using templates.  Anyway, a
>> macro is _already_ an improvement over the status quo, which is
>> open-coding the sizeof division[2].  Neither in C++ nor in C it is
>> idiomatic to write that division all the time; and it's actually quite
>> dangerous; more than macros, I'd say.
> 
> I agree with that.
> 
>> Heh!  I've been rewriting a big part of shadow in the last two years,
>> going from a partially-pre-ANSI code base to now C11 and POSIX.1-2008.
>> It resulted (so far) in a net removal of ~1 kLOC (+3k -4k), which is a
>> nice clean up.  I don't discard doing a similar clean-up in groff, if
>> certain C idioms are welcome.  :)
> 
> The amount of looping over arrays of pointers that is done is
> staggering.  So much of this could be killed off C++11's
> for (auto item : collection) idiom.
> 
> But I think want to understand the C++98 ships in the harbor a bit
> before I burn the fleet.

I'll send some C99 ships to try to convince you to get on board with
their brand new sails --pure cotton, or 99%--.

> 
> Anyone want to rewrite something in src/utils in Ada?  ;-)

Heh, not me.  :D

> 
> Regards,
> Branden

Cheers,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

OpenPGP_signature
Description: OpenPGP digital signature