<div dir="ltr"><div>Hi Howard</div><div> </div><div>I think, LIBCPP_FUNC_VIS might be the wrong thing to apply here (though I thought it was the right thing too initially).</div><div> </div><div>But after talking to Nico, it appears he (or someone) introduced _LIBCPP_NEW_DELETE_VIS especially for this case. It should if I understand Nico correctly resolve to the same thing as _LIBCPP_FUNC_VIS for APPLE etc. but for Windows it will be something different (probably nothing) where as _LIBCPP_FUNC_VIS will export the symbol and that isn't what we want for windows.</div>
<div> </div><div>Check out the <new> header. and you should be able to verify this for yourself.</div><div> </div><div>Thanks<br><br></div><div class="gmail_extra"><div class="gmail_quote">On Sat, Oct 5, 2013 at 1:00 PM, Howard Hinnant <span dir="ltr"><<a href="mailto:howard.hinnant@gmail.com" target="_blank">howard.hinnant@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;padding-left:1ex;border-left-color:rgb(204,204,204);border-left-width:1px;border-left-style:solid"><div><div>On Sep 25, 2013, at 9:42 AM, G M <<a href="mailto:gmisocpp@gmail.com" target="_blank">gmisocpp@gmail.com</a>> wrote:<br>


<br>
> Hi Everyone<br>
><br>
> The attached patch is for libcxx's new.cpp and __config files. The patch's intent is to make new.cpp compile using MS's cl.exe compiler without changing the meaning of anything for any other compiler.<br>


><br>
> The issue this patch seeks to address is that MS's compiler (cl.exe) doesn't support the __attribute__((__weak__)) or __atribute__((__visibility__("default")) syntax; so a solution must be found where cl.exe doesn't see this syntax.<br>


><br>
> This patch seeks to solve this problem by changing code patterned like this:<br>
> __attribute__((__weak__, __visibility__("default")))<br>
> void* operator new(size_t size, const std::nothrow_t&) _NOEXCEPT { /*snip*/; return p; }<br>
><br>
> to code like this:<br>
> _LIBCPP_WEAK<br>
> void* operator new(size_t size, const std::nothrow_t&) _NOEXCEPT { return p; }<br>
><br>
> with the expectation that this change will NOT introduce any functionality change for clang++/g++ etc. That expectation is based on two aspects of the change:<br>
><br>
> * The first is the belief that cl.exe doesn't support "weak" in any documented way and that libcxx on Windows doesn't need it anyway. So _LIBCPP_WEAK is defined as nothing when cl.exe is the detected compiler.<br>


><br>
> For all other compilers, _LIBCPP_WEAK is defined to be just __attribute__((__weak__)) and nothing more).<br>
> This should mean that cl.exe doesn't see the weak attribute syntax and so won't choke on it; and g++/clang++ will see the same weak attribute that it saw before this patch.<br>
><br>
> * The second part is what to do about __attribute__((_visibility__("default"))) as in the proposed change it is dropped from the function definition.<br>
><br>
> The expecatation here is that this is ok because it isn't neccessary because the prototype for the modified functions already have it; so the right thing should still happen.<br>
> If all of this is correct, then this patch should fix new.cpp for cl.exe without changing anything else.<br>
><br>
> It also provides a pattern that will work with all the compilers libcxx already supports; and without having to introduce alternate #if/#else guards or other uglyness. This should make it better match the patterns libcxx already uses.<br>


> If removing the "default" attribute turns out to be a problem, I believe the default attribute could be added back now that it is decoupled from the "weak" attribute (which I think is a good thing in of itself) by using one of libcxx's existing macro's such as _LIBCPP_FUNC_VIS / _LIBCPP_NEW_DELETE_VIS etc.<br>


><br>
> I'm not sure of the neccessity of LIBCPP_NEW_DELETE_VIS or it's realtionship to _LIBCPP_FUNC_VIS at this point, FWIW, but that doesn't matter to the logic of this patch.<br>
><br>
> I compiled this patch with cl.exe, g++ and clang++.exe.<br>
><br>
> Please let me know what you think. If this patch doesn't get traction, I'd appreciate some advice with real alternative code that could be used to advance things here as I found it hard to produce something actionable from the comments I received to my previous patch for this problem though I did and do appreciate the responses.<br>


><br>
> Thanks<br>
</div></div>> <libcxx_weak.diff><br>
<br>
Committed revision 192007.  See commit comments for minor modifications to the patch.<br>
<br>
Thanks,<br>
Howard<br>
<br>
</blockquote></div><br></div></div>