This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Clarify _LIBCPP_NEW_DELETE_VIS for Windows
ClosedPublic

Authored by smeenai on Sep 28 2016, 1:44 PM.

Details

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.

Diff Detail

Event Timeline

smeenai updated this revision to Diff 72889.Sep 28 2016, 1:44 PM
smeenai retitled this revision from to [libc++] Clarify _LIBCPP_NEW_DELETE_VIS for Windows.
smeenai updated this object.
smeenai added reviewers: compnerd, EricWF, mclow.lists.
smeenai added a subscriber: cfe-commits.
smeenai updated this revision to Diff 72891.Sep 28 2016, 1:47 PM

Clarifying code comment

EricWF accepted this revision.Sep 28 2016, 2:28 PM
EricWF edited edge metadata.

LGTM modulo inline comments.

include/new
129

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

This revision is now accepted and ready to land.Sep 28 2016, 2:28 PM
smeenai updated this revision to Diff 72911.Sep 28 2016, 3:12 PM
smeenai edited edge metadata.

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

This revision was automatically updated to reflect the committed changes.