[PATCH] Change base mutex implementations to use STL-provided mutexes

Aaron Ballman aaron.ballman at gmail.com
Fri Jun 6 13:15:46 PDT 2014


On Fri, Jun 6, 2014 at 4:10 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
>
> On Fri, Jun 6, 2014 at 12:32 PM, Alp Toker <alp at nuanti.com> wrote:
>>
>> Hi Zachary,
>>
>> In LLVM platform-specific code goes in the implementation files (.inc) and
>> never in the portable headers. Certainly for a high-level class like Mutex I
>> wouldn't expect to see platform-specific conditionals exposed.
>>
>> A couple of exceptions have slipped in like Support/FileSystem.h and
>> Support/Process.h with ifdef WIN32 but those are meant to get fixed at some
>> point.
>
>
> While you may be describing the current situation, I don't think that this
> is really a strong rule than many LLVM developers feel constrained by, and I
> wouldn't want it to be.
>
> I think the important rule is subtly but importantly different: the header
> files must, ultimately, be portable. Where possible, it is useful to isolate
> platform specific code away from header files, but it is not always possible
> or desirable. Either way, the header file needs to carefully manage any
> platform specifics involved to ensure it works correctly and reliably across
> platforms.
>
> One example of this (but not necessarily a *good* example -- I would use a
> different technique) is our DataTypes.h which is generated at
> configure/cmake time.
>
> I think it is entirely possible that the correct thing to do for some parts
> of Support is to include windows.h with appropriate control of macros and
> behind appropriate guards. I say it is "possible" because I don't know very
> much about the *specific* case of windows.h and its problems. Perhaps there
> are specific problems with it that make it intractable to use in this way
> even on Windows. But the general concept seems fine when useful. Similarly,
> if we wanted to include a Mac OS X header file to access a synchronization
> or other primitive provided by the OS, it would seem reasonable to include
> that header, again provided sufficient care and guards are used to make sure
> the header, as a whole, remains a reliable header for other projects to use
> on each platform.

Windows.h may be a special case because it's a catch-all header that
brings in a *lot* of stuff. As Reid pointed out, for instance, this
would break compilation of COFF.h.

For the time I've been doing Win32 work on Clang and LLVM, the policy
has been that Windows includes should live in the implementation
files, not in the header files. Of course, there are exceptions to
every rule -- I just don't think this one is an exception.

~Aaron



More information about the llvm-commits mailing list