[libcxx-commits] [PATCH] D58203: [libc++] Inline stdexcept constructors, destructors, and assignment operators when using MSVC ABI
Eric Fiselier via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Feb 14 00:26:44 PST 2019
EricWF added inline comments.
================
Comment at: include/stdexcept:83
explicit logic_error(const string&);
+#if defined(_LIBCPP_ABI_INLINE_STDEXCEPT_DEFINITIONS)
+ _LIBCPP_INLINE_VISIBILITY explicit logic_error(const char* __msg) : __imp_(__msg) {}
----------------
We should declare the same set of symbols regardless of `_LIBCPP_ABI_INLINE_STDEXCEPT_DEFINITIONS`.
And when we want inline definitions, instead of defining it inside the class, let's put an inline definition just below the class and guard that.
================
Comment at: src/include/refstring.h:69
-inline
+_LIBCPP_REFSTRING_INLINE_VISIBILITY
__libcpp_refstring::__libcpp_refstring(const char* msg) {
----------------
thomasanderson wrote:
> 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?
I don't understand that error at all.
The constructor it's saying is undefined is marked `inline`. If it is ODR-used in a translation unit, then MSVC needs to emit a definition for it in that TU.
Am I missing something?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58203/new/
https://reviews.llvm.org/D58203
More information about the libcxx-commits
mailing list