[libc++] Clarify _LIBCPP_NEW_DELETE_VIS for Windows

[smeenai]

Summary

Replace a stale reference to cxx_EXPORTS with _LIBCPP_BUILDING_LIBRARY,
and clarify why the operator new and delete family of functions are
marked dllexport when building but *not* dllimport when including the
header externally.

The new code is identical to the intent of the old code (and would be
functionally equivalent were cxx_EXPORTS still defined when building
libc++). The overall behavior is not ideal, since Microsoft's operator
new and delete functions will get called instead of libc++'s, but I
think consistently calling msvcrt's functions is better than either
calling msvcrt's or libc++'s functions depending on header inclusion.

[smeenai]

Clarifying code comment

[EricWF]

LGTM modulo inline comments.

include/new

129 ↗

(On Diff #72891)

Could you put this documentation in VisibilityMacros.rst instead of in the headers?

[smeenai]

Moving documentation to documentation file, per @EricWF's comment

libcxx/trunk/docs/DesignDocs/VisibilityMacros.rst

 
 **_LIBCPP_EXCEPTION_ABI**
   Mark the member functions, typeinfo, and vtable of the type as being exported
   by the libc++ library. This macro must be applied to all *exception types*.
   Exception types should be defined directly in namespace `std` and not the
   versioning namespace. This allows throwing and catching some exception types
   between libc++ and libstdc++.
 
+**_LIBCPP_NEW_DELETE_VIS**
+  Mark a symbol as being exported by the libc++ library. This macro must be
+  applied to all `operator new` and `operator delete` overloads.
+
+  **Windows Behavior**: When using the Microsoft CRT, all the `operator new` and
+  `operator delete` overloads are defined statically in `msvcrt.lib`. Marking
+  them as `dllimport` in the libc++ `<new>` header is therefore undesirable: if
+  we were to mark them as `dllimport` and then link against libc++, source files
+  which included `<new>` would end up linking against libc++'s `operator new`
+  and `operator delete`, while source files which did not include `<new>` would
+  end up linking against msvcrt's `operator new` and `operator delete`, which
+  would be a confusing and potentially error-prone inconsistency.
+
 Links
 =====
 
 * `[cfe-dev] Visibility in libc++ - 1 <http://lists.llvm.org/pipermail/cfe-dev/2013-July/030610.html>`_
 * `[cfe-dev] Visibility in libc++ - 2 <http://lists.llvm.org/pipermail/cfe-dev/2013-August/031195.html>`_
 * `[libcxx] Visibility fixes for Windows <http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130805/085461.html>`_

libcxx/trunk/include/new

 struct _LIBCPP_TYPE_VIS nothrow_t {};
 extern _LIBCPP_FUNC_VIS const nothrow_t nothrow;
 typedef void (*new_handler)();
 _LIBCPP_FUNC_VIS new_handler set_new_handler(new_handler) _NOEXCEPT;
 _LIBCPP_FUNC_VIS new_handler get_new_handler() _NOEXCEPT;
 
 }  // std
 
-#if defined(_WIN32) && !defined(cxx_EXPORTS)
-# define _LIBCPP_NEW_DELETE_VIS _LIBCPP_FUNC_VIS_ONLY
+#if defined(_LIBCPP_MSVCRT) && !defined(_LIBCPP_BUILDING_LIBRARY)
+# define _LIBCPP_NEW_DELETE_VIS
 #else
 # define _LIBCPP_NEW_DELETE_VIS _LIBCPP_FUNC_VIS
 #endif
 
 _LIBCPP_NEW_DELETE_VIS void* operator new(std::size_t __sz)
 #if !__has_feature(cxx_noexcept)
     throw(std::bad_alloc)
 #endif