[patch] [libcxx] _LIBCPP_WEAK

[G M]

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 <howard.hinnant at gmail.com>wrote:

> On Sep 25, 2013, at 9:42 AM, G M <gmisocpp at gmail.com> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131005/c78127ca/attachment.html>