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