<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 6, 2014 at 5:46 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br>
On 07/06/2014 02:34, Zachary Turner wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Remove the Mutex.h dependency on windows.h, and address various other review comments.<br>
<br>
This solution is still less than ideal, because we need to include some windows preprocessor checks inside of Mutex.h.<br>
</blockquote>
<br></div>
It seems you could just have CriticalSectionMutex implemented as a standard Mutex when building on Unix.<br>
<br>
That way you don't need any preprocessor conditionals in either the header definition or at the point of use, and the fallback on non-Windows platforms is seamless.</blockquote><div>But a Critical Section is not a thing that exists on Unix.  Isn't it a little strange to have a name referring to something that only exists on one platform just for convenience?</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The best solution in my opinion would require allowing some platform specific headers to be in the include tree  (for example, include\llvm\Support\Windows\<u></u>CriticalSectionMutex.h).  This would remove all of the preprocessor code from Mutex.h, and allow anyone who needs this to simply write #include "llvm/Support/Windows/<u></u>CriticalSectionMutex.h".<br>

</blockquote>
<br></div>
We have actual clients we need to support that break with windows.h due to stomping on global namespaces and definitions -- conflicts are a likely scenario with any non-trivial piece of code that wasn't written to target Windows originally.<br>

<br>
The elegant and efficient solutions are all around if you take a look, but exposing windows.h isn't one of them.</blockquote><div><br></div><div>Actually the solution I described there does not require exposing windows.h through a header.  I understand that is frought with peril, and although I do think we can (and should) eventually try to achieve that, I also understand that it is difficult, requires care, and is outside the scope of this CL.</div>
<div><br></div><div>Instead, what I'm saying is that we could simply expose some platform specific headers to people outside of LLVM.   These headers themselves would not include platform specific headers, they would only expose opaque classes that deal with platform specific functionality.  For example, this "include\Support\Windows\CriticalSectionMutex.h" would just have a void* member, and lib\Support\Windows\CriticalSectionMutex.cpp would implement it.  then libclang could #include "Support\Windows\CriticalSectionMutex.h"</div>
</div></div></div>