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 <a...@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-db70262b6...@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-ab25423ee...@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
