Ok, thanks. Committed revision 192071. Howard
On Oct 4, 2013, at 8:34 PM, G M <[email protected]> wrote: > Hi Howard > > I think, LIBCPP_FUNC_VIS might be the wrong thing to apply here (though I > thought it was the right thing too initially). > > 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. > > Check out the <new> header. and you should be able to verify this for > yourself. > > Thanks > > On Sat, Oct 5, 2013 at 1:00 PM, Howard Hinnant <[email protected]> > wrote: > On Sep 25, 2013, at 9:42 AM, G M <[email protected]> wrote: > > > Hi Everyone > > > > 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. > > > > 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. > > > > This patch seeks to solve this problem by changing code patterned like this: > > __attribute__((__weak__, __visibility__("default"))) > > void* operator new(size_t size, const std::nothrow_t&) _NOEXCEPT { > > /*snip*/; return p; } > > > > to code like this: > > _LIBCPP_WEAK > > void* operator new(size_t size, const std::nothrow_t&) _NOEXCEPT { return > > p; } > > > > 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: > > > > * 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. > > > > For all other compilers, _LIBCPP_WEAK is defined to be just > > __attribute__((__weak__)) and nothing more). > > 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. > > > > * The second part is what to do about > > __attribute__((_visibility__("default"))) as in the proposed change it is > > dropped from the function definition. > > > > 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. > > If all of this is correct, then this patch should fix new.cpp for cl.exe > > without changing anything else. > > > > 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. > > 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. > > > > 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. > > > > I compiled this patch with cl.exe, g++ and clang++.exe. > > > > 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. > > > > Thanks > > <libcxx_weak.diff> > > Committed revision 192007. See commit comments for minor modifications to > the patch. > > Thanks, > Howard > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
