[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-29 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini closed this revision. mehdi_amini added a comment. r299052 ; let me know if you want to improve the comment in any way. https://reviews.llvm.org/D31272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-29 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments. Comment at: lib/CMakeLists.txt:155 +# We can't use the "-reexported_symbols_list" when we build the +# new/delete operators as part of the dylib: the linker would fail. +set(OSX_RE_EXPORT_LINE

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-29 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF accepted this revision. EricWF added inline comments. This revision is now accepted and ready to land. Comment at: lib/CMakeLists.txt:155 +# We can't use the "-reexported_symbols_list" when we build the +# new/delete operators as part of the dylib: the

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-28 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment. OK, I figured, it is because I have this revision locally on top of this one: https://reviews.llvm.org/D30765 ; and I can't submit the latter without the change here. https://reviews.llvm.org/D31272 ___ cfe-commits

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-27 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment. In https://reviews.llvm.org/D31272#711877, @EricWF wrote: > In https://reviews.llvm.org/D31272#711876, @mehdi_amini wrote: > > > Strange. So installing the command line tools is not enough. It has to be > > that CMAKE_OSX_SYSROOT is only defined on Apple internal

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-27 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added a comment. In https://reviews.llvm.org/D31272#711876, @mehdi_amini wrote: > Strange. So installing the command line tools is not enough. It has to be > that CMAKE_OSX_SYSROOT is only defined on Apple internal install maybe? Are you sure you're starting with a clean CMake build

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-27 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment. Strange. So installing the command line tools is not enough. It has to be that CMAKE_OSX_SYSROOT is only defined on Apple internal install maybe? Anyway, if you're exporting through the ABI list and have LIBCXX_ENABLE_NEW_DELETE_DEFINITIONS I expect the link to

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-27 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added a comment. In https://reviews.llvm.org/D31272#711860, @mehdi_amini wrote: > In https://reviews.llvm.org/D31272#711804, @EricWF wrote: > > > I'm a bit confused by the description of this change. Libc++ has been > > enabling the new/delete definitions in its dylib since forever and

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-27 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment. Not that I'm claiming to understand why we're changing strategy when we have the command line tools installed or not in this case ;) (CC @beanz if he know by any chance!) https://reviews.llvm.org/D31272 ___

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-27 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment. In https://reviews.llvm.org/D31272#711804, @EricWF wrote: > I'm a bit confused by the description of this change. Libc++ has been > enabling the new/delete definitions in its dylib since forever and I've never > experienced a link error. Did you mean to say the

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-27 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added a comment. I'm a bit confused by the description of this change. Libc++ has been enabling the new/delete definitions in its dylib since forever and I've never experienced a link error. Did you mean to say the link error occurs only when libc++abi doesn't define them?

[PATCH] D31272: Do not pass an explicit reexported symbol list when building libc++ dylib if also defining new/delete

2017-03-22 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini created this revision. Herald added a subscriber: mgorny. The linker would fail because the list of reexported symbols contains new/delete operators. https://reviews.llvm.org/D31272 Files: lib/CMakeLists.txt Index: lib/CMakeLists.txt