[libcxx-commits] [PATCH] D58203: [libc++] Inline stdexcept constructors, destructors, and assignment operators when using MSVC ABI

[Tom Anderson via Phabricator via libcxx-commits]

thomasanderson marked an inline comment as not done.
thomasanderson added a comment.

In D58203#1397191 <https://reviews.llvm.org/D58203#1397191>, @EricWF wrote:

> hanks for looping me in. I'm working on a similar problem but with libstdc++.
>
> I think the approach of creating specific visibility macros for the exception / runtime bits than need to be shared makes sense,
>  though the additional complexity is unfortunate.


The visibility macro is only used in one place.  maybe we could just do something like:

  class
  #if defined(_LIBCPP_ABI_INLINE_STDEXCEPT_DEFINITIONS)
  _LIBCPP_TYPE_VIS
  #else
  _LIBCPP_HIDDEN
  #endif
  __libcpp_refstring

wdyt?



================
Comment at: src/include/refstring.h:69
 
-inline
+_LIBCPP_REFSTRING_INLINE_VISIBILITY
 __libcpp_refstring::__libcpp_refstring(const char* msg) {
----------------
EricWF wrote:
> These changes seem wrong. And I don't understand why the changes are needed in relation to MSVC.
This is now needed eg. for the inline definition of logic_error::logic_error(), which must now construct a __libcpp_refstring.

If I leave "inline" in, I get link errors:
    lld-link: error: undefined symbol: "public: __cdecl std::__1::__libcpp_refstring::__libcpp_refstring(char const *)" (??0__libcpp_refstring at __1@std@@QEAA at PEBD@Z)
    >>> referenced by obj/buildtools/third_party/libc++/libc++/string.obj

include/refstring.h gets included in stdexcept.cpp.  Actually, that's the only place it gets included, so maybe we could get rid of the inline altogether since we can't have multiple definitions?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58203/new/

https://reviews.llvm.org/D58203