Page MenuHomePhabricator

Make WindowsSupport.h a public header
AcceptedPublic

Authored by zturner on Jul 2 2018, 5:32 PM.

Details

Summary

I sort of hesitate to do this, but I honestly don't have a better solution. Currently WindowsSupport.h is kept private by putting it in llvm\lib\Support\Windows so that it's only available to use from within the support library itself, and only from implementation files.

However, it's not just the thing that sets up and invokes #include <windows.h>, it also has various utility functions and classes that wrap Windows specific types (e.g. a ScopedHandle). It would be nice to be able to share this kind of thing across other sub-projects and/or libraries (basically, anything not Support) when they have to invoke some platform specific code and end up needing the same thing. It would be nice if LLVM had some means of exposing platform specific utility functions, classes, and helpers in the same way that it exposes platform-independent ones.

The alternative is to just copy the stuff I need from WindowsSupport.h into another project-specific header. This would duplicate some code, but I won't blame anyone if they think that's better than making this header public.

Diff Detail

Event Timeline

zturner created this revision.Jul 2 2018, 5:32 PM

Will making it public cause more TUs to transitively include <windows.h>, or does that happen anyway?

I have no objections if the toothpaste is already out of the tube. But if it's not, I'd prefer to keep <windows.h> as contained as possible, since it defines so many macros and global names that can easily conflict.

Is there a good reason the use cases you want this for can't be added to libSupport? I'm a bit worried about continuing to add platform specific code outside of libSupport.

Is there a good reason the use cases you want this for can't be added to libSupport? I'm a bit worried about continuing to add platform specific code outside of libSupport.

Mostly that it's quite large. See my recent post on llvm-dev about RFC: libtrace. I don't want to develop an entire library inside of libSupport, and something like a libtrace will unavoidably have a lot of platform specific code. As another example, consider lldb. There's no way to avoid a bunch of platform specific code, and it would be nice if LLVM had a solution that allowed LLDB to re-use some of LLVM's own platform specific code.

Bigcheese accepted this revision.Jul 3 2018, 11:58 PM

Is there a good reason the use cases you want this for can't be added to libSupport? I'm a bit worried about continuing to add platform specific code outside of libSupport.

Mostly that it's quite large. See my recent post on llvm-dev about RFC: libtrace. I don't want to develop an entire library inside of libSupport, and something like a libtrace will unavoidably have a lot of platform specific code. As another example, consider lldb. There's no way to avoid a bunch of platform specific code, and it would be nice if LLVM had a solution that allowed LLDB to re-use some of LLVM's own platform specific code.

Yeah, thought that may be the reason. LGTM, although would it be reasonable to add a comment to the header saying not to include it in another non-windows-specific header?

This revision is now accepted and ready to land.Jul 3 2018, 11:58 PM