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

[Alejandro Colomar]

Hi Branden!

On 2023-08-04 06:32, G. Branden Robinson wrote:
> 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.
> [...]
>> I added it there because I didn't find a "common utilities" header
>> file.  Please suggest a better place.
> 
> 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 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 also dislike the idea of C++ deprecating macros.  IMO, they had to do
it because they bloated the language so much with dubious features that
make macros (and other C features) be unsafe.  But not because macros or
other features are inherently bad in C.  Yes, you can write unsafe
macros in C; but so you can write unsafe functions in C.  When written
by a careful programmer, macros can actually improve safety[3].

> 
> Yes, groff does use the preprocessor intensely in a couple of places.
> 
> https://git.savannah.gnu.org/cgit/groff.git/tree/src/include/itable.h?h=1.23.0
> https://git.savannah.gnu.org/cgit/groff.git/tree/src/include/ptable.h?h=1.23.0
> 
> That is because C++ didn't have templates, or they weren't yet mature[1]
> when James Clark wrote this stuff.
> 
> And I recognize that "cultivation of robust and flourishing macros" _is_
> (or can be) idiomatic C, as Ben Klemens champions.
> 
> groff is stuck in a very hard place.  People have lots of excuses to not
> hack on it.
> 
> 1.  From C programmers: "Eww! It's in C++!"
> 2.  From C++ programmers (1): "Eww!  It's not in C++23 already!  How can
>     you not be using all the latest features?  What a bunch of losers!"
> 3.  From people generally: "Why care about groff?  Or text formatting?
>     I want my Markdown documents in angry fruit salad colors on my
>     JavaScript terminal emulator!"
> 4.  From C++ programmers (2): "Wow, this pre-standard dialect is about
>     as mind-twisting to current practicioners as an exhibit of
>     pre-Typesetter C[1] (attached) is to C programmers."
> 
> I don't think coders in the first three categories can ever be won over
> short of a ground-up rewrite.  That _would_ be a cool project, and it's
> one Ralph Corderoy tried to get me to do (possibly so I'd stop posting
> to this mailing list).

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

>  But I kind of like working on code that people
> other than me are actually using.
> 
> At 2023-08-04T03:00:12+0200, Alejandro Colomar wrote:
>> *  src/roff/troff/env.cpp (is_family_valid): The old code was
>>    eyeball-bleeding.  This cuts 6 lines to 3, and each of them is
>>    significantly simpler to read.  Remove the comments of how it could
>>    be improved using modern C++, as I don't think it would improve much
>>    vs this C implementation.
> [...]
>> I need a new pair of eyeballs.  They bleeded so much!
> 
> Yeah, who's responsible for this crap?
> 

[... blame Branden ...]

> 
> This is a relatively new function, and all my fault.  I wrote it because
> validation of font families wasn't being done previously.
> 
> https://savannah.gnu.org/bugs/index.php?64155
> 
> In my defense, I seem to remember modeling it on an example in Lippman's
> _Essential C++_, which documents C++98.  I figured he was an expert.
> Bad choice?

If you ask me, C++ is a bad choice in general.  I don't know that book.

> 
> Lippman never updated that book for any later version of C++, and I
> don't know why.  Maybe everybody else decided his code was horrible.
> Or maybe he got off the C++ train before Scott Meyers did.[2]
> 
> But I put those C++11 comments in there because the C++98 way of doing
> things seemed incredibly gross,

It was.

> and I had trouble believing that C++
> programmers subjected themselves to it.  They did, and they didn't like
> it, but it took 13 years to get a remedy

Indeed.

> because, by God, _someone_ had
> to have "concepts" in the next version of language.[3]
> 
> Therefore--please take another crack at it without using the
> preprocessor, if you'd like.

I'm going to push back to you with this patch.  Please reconsider.

> 
> Regards,
> Branden
> 
> [1] "Still aren't!" <drum fill>

And in the opposite field, (GNU) C is very mature today.  We have
-fanalyzer, _FORTIFY_SOURCE, and so many tools that make it a robust
language; something that C++ is decades away from becoming.

> [2] https://scottmeyers.blogspot.com/2015/12/good-to-go.html
> [3] And failed to do so.  But I'm sure it pitches brilliantly in any
>     conference room.  All you have to do is stand up in front of a
>     whiteboard, put your elbows to your waist while extending your
>     forearms at a 45 degree angle in the plane parallel to the floor,
>     with palms facing up, gravely intone the word "concepts", and take
>     your seat.  Boom!  Instant buy-in!  Steve Jobs himself could not
>     have sold it better.

Cheers,
Alex


[1]:  Naive lengthof() is safe (when not in a library)

$ cat ptr.c 
#define lengthof(a)  (sizeof(a) / sizeof(*a))

int
main(void)
{
        int *p;
        return lengthof(p);
}

$ cc -Wall ptr.c 
ptr.c: In function ‘main’:
ptr.c:1:33: warning: division ‘sizeof (int *) / sizeof (int)’ does not compute 
the number of array elements [-Wsizeof-pointer-div]
    1 | #define lengthof(a)  (sizeof(a) / sizeof(*a))
      |                                 ^
ptr.c:8:16: note: in expansion of macro ‘lengthof’
    8 |         return lengthof(p);
      |                ^~~~~~~~
ptr.c:6:14: note: first ‘sizeof’ operand was declared here
    6 |         int *p;
      |              ^

      And with g++:

$ g++ -Wall ptr.c 
ptr.c: In function ‘int main()’:
ptr.c:1:33: warning: division ‘sizeof (int*) / sizeof (int)’ does not compute 
the number of array elements [-Wsizeof-pointer-div]
    1 | #define lengthof(a)  (sizeof(a) / sizeof(*a))
      |                       ~~~~~~~~~~^~~~~~~~~~~~
ptr.c:8:16: note: in expansion of macro ‘lengthof’
    8 |         return lengthof(p);
      |                ^~~~~~~~
ptr.c:6:14: note: first ‘sizeof’ operand was declared here
    6 |         int *p;
      |              ^


[2]:  Currently, groff extensively open-codes the sizeof division

$ grep -rn -e 'sizeof.*/ *sizeof.*\*' -e 'sizeof.*/ *sizeof.*\[0]' \
| grep -v gnulib \
| wc -l;
39


[3]:  I designed these malloc(3) wrappers that are a lot safer than malloc(3):

      <https://github.com/shadow-maint/shadow/blob/master/lib/alloc.h>

      Check the following commits in the shadow git repository, which document
      why and how these macros are much safer than plain malloc(3):

      *  09775d37 Simplify allocation APIs
      *  efbbcade Use safer allocation macros
      *  6e58c127 libmisc: Add safer allocation macros

      <https://github.com/shadow-maint/shadow/commit/09775d37>
      <https://github.com/shadow-maint/shadow/commit/efbbcade>
      <https://github.com/shadow-maint/shadow/commit/6e58c127>


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

OpenPGP_signature
Description: OpenPGP digital signature