#25752 closed enhancement (fixed)

Detangle our included headers and reduce reliance on or.h

Reported by: isis Owned by:
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-modularity, tor-compatibility, static-analysis
Cc: Actual Points:
Parent ID: Points: 5
Reviewer: Sponsor: Sponsor8-can

Description (last modified by isis)

Over the years, our code has grown to have a lot of #includes which are no longer necessary, and others which are over the scope of what actually needs to be included. A prime example of this is that nearly everything in /src/or includes or.h which is a huge, insanely long header file with nearly every type we've ever made. It'd be nice if we could decouple things a bit more.

I made brief foray into playing with automated tools for doing this last week. First I tried https://github.com/myint/cppclean, but it wasn't able to process any of our code without displaying a python traceback, so I moved on to https://include-what-you-use.org/. iwyu worked a bit better, but it really wanted to change system headers and remove the defined safeguards (e.g. #ifdef HAVE_SYS_SOCKET_H #include <sys/socket.h> #endif), which feels super scary and I'm pretty sure will result in a lot of weird breakage, particularly on systems of lower tier support status and systems developers tend to not use (e.g. win32). There's a bunch of pragmas (https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md) we could probably use to tell it to leave alone the system headers (e.g. find src -iname "*.[ch]" -exec sed -i -e 's/#include <*>/#include <*> \/\/ IYWU pragma: keep/' {} \; or something).

It also wanted to do things like removing an #include or.h and then including from that only the system headers that were used, so maybe it's also a good idea to split up or.h? Like we could put the system headers and compatibility stuff at the top into a prelude.h or something that every module includes, and then more specific stuff in other header files. It might be nice to map out or graph which sections of code tend to need to use the same headers, in order to facilitate the organisation of splitting up or.h.

I have a feature/iwyu-test branch where I committed the changes that iwyu made, plus some manual fixups that I made as I was perusing the changes (sorry, I should have kept those separate probably). I had to basically discard all changes to the ref10 ed25519 implementation, because it didn't understand why there were #includes mid-C-function. Also it wanted to #include <bits/socket_type.h> in several places, which had to be replaced with #include <sys/socket.h> instead, no idea why it wanted the private header in the first place (also if it did, why it didn't also #define _SYS_SOCKET_H). The output of configure --disable-asciidoc && make -k CC=/home/isis/code/sources/iwyu/build/include-what-you-use 2>/tmp/iwyu.out is attached here. Also I compiled iwyu against LLVM/clang 3.5.

Child Tickets

Change History (3)

comment:1 Changed 14 months ago by isis

Grrr, trac won't allow a file that large. The output is here: https://people.torproject.org/~isis/docs/2018-04-09-bug25752-iwyu-output.txt

comment:2 Changed 14 months ago by isis

Description: modified (diff)

comment:3 Changed 12 months ago by dgoulet

Resolution: fixed
Status: newclosed

Epic work happened by nickm in the last weeks on refactoring src/common/ and in the process or.h

Note: See TracTickets for help on using tickets.