[libc++] Correct explanation of _LIBCPP_NEW_DELETE_VIS

[smeenai]

Summary

The behavior of this macro actually needs to apply universally on
Windows and not just when using the Microsoft CRT. Update the macro
definition and documentation accordingly.

[smeenai]

My initial understanding of the problem was incorrect; I did some further experimentation and I think I properly understand what's going on here now. Sorry for the churn.

[mclow.lists]

Sigh. Make an expedient choice that you don't really agree with, and you get immediately reminded of it. I suggested on an earlier review (not this patch) that I really didn't want to see _WIN32 in any files other than <config>, that we should have a libc++-specific one. I let that slide because we had a couple other instances of _WIN32 in the source base.

About three days later, this pops up. :-(
Not your fault, but it makes me sad.

[smeenai]

In D25145#565027, @mclow.lists wrote:

Sigh. Make an expedient choice that you don't really agree with, and you get immediately reminded of it. I suggested on an earlier review (not this patch) that I really didn't want to see _WIN32 in any files other than <config>, that we should have a libc++-specific one. I let that slide because we had a couple other instances of _WIN32 in the source base.

About three days later, this pops up. :-(
Not your fault, but it makes me sad.

I'm not a fan either :) Gonna commit this and then send something to cfe-dev about cleaning them up.

[smeenai]

Not sure why Phabricator isn't closing this out, but this was committed as SVN r284016

docs/DesignDocs/VisibilityMacros.rst

**_LIBCPP_EXCEPTION_ABI**
   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.
+  **Windows Behavior**: The `operator new` and `operator delete` overloads
+  should not be marked as `dllimport`; if they were, source files including the
+  `<new>` header (either directly or transitively) would lose the ability to use
+  local overloads of `operator new` and `operator delete`. On Windows, this
+  macro therefore expands to `__declspec(dllexport)` when building the library
+  and has an empty definition otherwise. A related caveat is that libc++ must be
+  included on the link line before `msvcrt.lib`, otherwise Microsoft's
+  definitions of `operator new` and `operator delete` inside `msvcrt.lib` will
+  end up being used instead of libc++'s.
 
 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>`_

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(_LIBCPP_MSVCRT) && !defined(_LIBCPP_BUILDING_LIBRARY)
+#if defined(_WIN32) && !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)