Config.h

It would be useful if there were a header file that were always included in every CPP file before any other header file, that would initialize any platform-specific defines and whatnot. We already have a header file that seems to serve exactly this purpose, but it just isn’t always included. It’s lldb/Host/Config.h

Doing a single pass over every file and manually including this would be a lot of work, but can we make it a policy going forward that a) this file will always be included first, in every newly created cpp file, and b) When touching files, try to remember to add this include at the top?

Why wouldn’t you just include it in every file where it’s needed? Why does it have to be included first?

This seems a little backward to me. We should be encouraging all the generic code in lldb to have no host dependencies unless they are absolutely necessary, shouldn't we? Making all .cpp files include a host specific header file seems going counter to this aim.

Jim

Even simple typedefs already imply a host dependency. Currently platform specific typedefs are not brought in through Host/Config.h, but they could be for example. But I guess the point is that if you’re compiling for a particular host, you’re already married to it.

I guess I only find this an issue because the codebase or style guide doesn’t seem to discourage using #ifdefs for platform-specific logic. So say I’m editing some file, and I see the use of LLDB_DISABLE_POSIX. I then go use that same define in another file because I need it, and it’s not defined because Config.h isn’t included. Including it is obviously an easy fix, but it just strikes me as inconsistent that it would be defined in some places and not others. What if someone uses it in a header file? (Note that it’s already used in a header file, btw. ProcessLaunchInfo.h).

FWIW, I would encourage LLDB to look at how LLVM does platform-specific
stuff... It might need some tweaking (I generally expect there to be more
platform specific code in LLDB than LLVM proper) but the patterns should be
pretty reasonable to follow IMO. (And we don't have a must-include-first
config header, which is I think a design mistake. Consider what happens if
we switch to a modules-based LLVM and LLDB build...)

In general I look suspiciously at all uses of #ifdef SOME_HOST_THING.

In some cases, like ConnectionFileDescriptor where this is really a host functionality, but there's enough common code that it wasn't worthwhile to try to tease out into host specific subclasses, it's kinda okay to do it this way. More of a perfect is the enemy of the good type thing.

But for instance the fact that ProcessLaunchInfo has a method that #ifdef'ed LLDB_DISABLE_POSIX means somebody was taking a shortcut. Again, time and the need for forward progress being what they are, maybe that's an okay tradeoff. But you should probably mentally give yourself a demerit for doing it at least...

Jim