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

[G. Branden Robinson]

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.

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

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

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.

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

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

Regards,
Branden

signature.asc
Description: PGP signature