Skip to content

Commit

Permalink
Check for full thread_local support.
Browse files Browse the repository at this point in the history
When building on clang with libcxxrt, @borischikunov reports that
non-POD `thread_local` objects will fail to link.  Luckily our only use
of `thread_local` at the moment is as an optimisation.  So, check for
full support and if it's not present, forego the optimisation.

Fixes #262.
  • Loading branch information
jtv committed Jan 28, 2020
1 parent fa0567d commit 1ef49d1
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 12 deletions.
4 changes: 4 additions & 0 deletions cmake/config.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ try_compile(
PQXX_HAVE_CHARCONV_INT
${PROJECT_BINARY_DIR}
SOURCES ${PROJECT_SOURCE_DIR}/config-tests/charconv_int.cxx)
try_compile(
PQXX_HAVE_THREAD_LOCAL
${PROJECT_BINARY_DIR}
SOURCES ${PROJECT_SOURCE_DIR}/config-tests/thread_local.cxx)

# check_cxx_source_compiles requires CMAKE_REQUIRED_DEFINITIONS to specify
# compiling arguments.
Expand Down
10 changes: 10 additions & 0 deletions config-tests/thread_local.cxx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Test for std::to_string/std::from_string for floating-point types.
#include <iostream>
#include <sstream>

int main(int argc, char **)
{
thread_local std::stringstream s;
s << argc;
std::cout << s.str();
}
1 change: 1 addition & 0 deletions configitems
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ PQXX_HAVE_POLL internal compiler
PQXX_HAVE_PQENCRYPTPASSWORDCONN internal libpq
PQXX_HAVE_STRNLEN public compiler
PQXX_HAVE_STRNLEN_S public compiler
PQXX_HAVE_THREAD_LOCAL private compiler
VERSION internal autotools
41 changes: 41 additions & 0 deletions configure
Original file line number Diff line number Diff line change
Expand Up @@ -18754,6 +18754,47 @@ rm -f core conftest.err conftest.$ac_objext \
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $have_charconv_float" >&5
$as_echo "$have_charconv_float" >&6; }

# As per #262, clang with libcxxrt does not support thread_local on non-POD
# objects. Luckily we can live without those, it's just less efficient.
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for full thread_local support" >&5
$as_echo_n "checking for full thread_local support... " >&6; }
have_thread_local=yes
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
/* end confdefs.h. */
// Test for std::to_string/std::from_string for floating-point types.

#include <iostream>

#include <sstream>



int main(int argc, char **)

{

thread_local std::stringstream s;

s << argc;

std::cout << s.str();

}


_ACEOF
if ac_fn_cxx_try_link "$LINENO"; then :

$as_echo "#define PQXX_HAVE_THREAD_LOCAL 1" >>confdefs.h

else
have_thread_local=no
fi
rm -f core conftest.err conftest.$ac_objext \
conftest$ac_exeext conftest.$ac_ext
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $have_thread_local" >&5
$as_echo "$have_thread_local" >&6; }

# Doing my own test for poll() for now. The test built into autoconf-archive
# triggers stupid warnings.
# TODO: Try newer version of autoconf-archive.
Expand Down
13 changes: 13 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,19 @@ AC_LINK_IFELSE(
have_charconv_float=no)
AC_MSG_RESULT($have_charconv_float)

# As per #262, clang with libcxxrt does not support thread_local on non-POD
# objects. Luckily we can live without those, it's just less efficient.
AC_MSG_CHECKING([for full thread_local support])
have_thread_local=yes
AC_LINK_IFELSE(
[read_test(thread_local.cxx)],
AC_DEFINE(
[PQXX_HAVE_THREAD_LOCAL],
1,
[Define if thread_local is fully supported.]),
have_thread_local=no)
AC_MSG_RESULT($have_thread_local)

# Doing my own test for poll() for now. The test built into autoconf-archive
# triggers stupid warnings.
# TODO: Try newer version of autoconf-archive.
Expand Down
3 changes: 3 additions & 0 deletions include/pqxx/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@
/* Define if compiler provides strnlen_s */
#undef PQXX_HAVE_STRNLEN_S

/* Define if thread_local is fully supported. */
#undef PQXX_HAVE_THREAD_LOCAL

/* Define to 1 if you have the ANSI C header files. */
#undef STDC_HEADERS

Expand Down
68 changes: 56 additions & 12 deletions src/strconv.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@

namespace
{
/// Do we have fully functional thread_local support?
/** When building with libcxxrt on clang, you can't create thread_local objects
* of non-POD types. Any attempt will result in a link error.
*/
constexpr bool have_thread_local
{
#if defined(PQXX_HAVE_THREAD_LOCAL)
true
#else
false
#endif
};

/// String comparison between string_view.
constexpr inline bool equal(std::string_view lhs, std::string_view rhs)
{
Expand Down Expand Up @@ -445,6 +458,15 @@ template<typename T> class dumb_stringstream : public std::stringstream
};


template<typename F> inline
bool from_dumb_stringstream(
dumb_stringstream<F> &s, F &result, std::string_view text)
{
s.str(std::string{text});
return static_cast<bool>(s >> result);
}


// These are hard, and popular compilers do not yet implement std::from_chars.
template<typename T> inline T from_string_awful_float(std::string_view text)
{
Expand Down Expand Up @@ -476,14 +498,21 @@ template<typename T> inline T from_string_awful_float(std::string_view text)
}
else
{
thread_local dumb_stringstream<T> S;
// Visual Studio 2017 seems to fail on repeated conversions if the
// clear() is done before the seekg(). Still don't know why! See #124
// and #125.
S.seekg(0);
S.clear();
S.str(std::string{text});
ok = static_cast<bool>(S >> result);
if (have_thread_local)
{
thread_local dumb_stringstream<T> S;
// Visual Studio 2017 seems to fail on repeated conversions if the
// clear() is done before the seekg(). Still don't know why! See #124
// and #125.
S.seekg(0);
S.clear();
ok = from_dumb_stringstream(S, result, text);
}
else
{
dumb_stringstream<T> S;
ok = from_dumb_stringstream(S, result, text);
}
}
break;
}
Expand Down Expand Up @@ -558,6 +587,15 @@ template char *
float_traits<long double>::into_buf(char *, char *, long double const &);


template<typename F> inline
std::string to_dumb_stringstream(dumb_stringstream<F> &s, F value)
{
s.str("");
s << value;
return s.str();
}


/// Floating-point implementations for @c pqxx::to_string().
template<typename T> std::string to_string_float(T value)
{
Expand All @@ -576,10 +614,16 @@ template<typename T> std::string to_string_float(T value)
// In this rare case, we can convert to std::string but not to a simple
// buffer. So, implement to_buf in terms of to_string instead of the other
// way around.
thread_local dumb_stringstream<T> s;
s.str("");
s << value;
return s.str();
if (have_thread_local)
{
thread_local dumb_stringstream<T> s;
return to_dumb_stringstream(s, value);
}
else
{
dumb_stringstream<T> s;
return to_dumb_stringstream(s, value);
}
}
#endif
}
Expand Down

0 comments on commit 1ef49d1

Please sign in to comment.