clang: async_resolve incompatible with string_view

[qis]

When compiling with MSVC (2017 and 2017.3), the following code works just fine. If I try to compile it with LLVM/Clang (4.0.1 with libcxx), I have to replace std::string_view with std::string.

#include <exception>
#include <iostream>
#include <net>
#include <string_view>
#include <system_error>

int main(int argc, char* argv[]) {
  try {
    // Must be std::string for clang.
    const std::string_view address = argc > 1 ? argv[1] : "localhost";
    const std::string_view service = argc > 2 ? argv[2] : "8080";

    std::net::io_context io_context;
    std::net::ip::tcp::socket socket(io_context);
    std::net::ip::tcp::resolver resolver(io_context);

    using results_type = std::net::ip::tcp::resolver::results_type;
    resolver.async_resolve(address, service, [&](const std::error_code& ec, results_type results) {
      if (ec) {
        throw std::system_error(ec, "resolve");
      }
      // Cannot use results->endpoint() because of private inheritance.
      std::cout << results.begin()->endpoint() << std::endl;
    });

    io_context.run();
  }
  catch (const std::exception& e) {
    std::cerr << e.what() << std::endl;
  }
  return 0;
}

The error is caused by NET_TS_HAS_STD_STRING_VIEW being set to 0.

Here is the <net> header:

//
// net
// ~~~~~~~~~~~~~~~~
//
// Copyright (c) 2003-2017 Christopher M. Kohlhoff (chris at kohlhoff dot com)
//
// Distributed under the Boost Software License, Version 1.0. (See accompanying
// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
//

#ifndef NET_TS_NET_INCLUDED
#define NET_TS_NET_INCLUDED

#include <experimental/__net_ts/ts/net.hpp>

namespace std {

namespace net = experimental::net;

}  // namespace std

#endif // NET_TS_NET_INCLUDED

P.S.: Is it intentional that basic_resolver_results inherits privately from basic_resolver_iterator? Should I create a second issue regarding this?
P.P.S.: Great implementation even if it's "just" converted from ASIO. I even see a slight performance increase (~5%). Thank you!

[qis]

And here is a patch to enable <string_view> support.

diff --git i/include/experimental/__net_ts/detail/config.hpp w/include/experimental/__net_ts/detail/config.hpp
--- i/include/experimental/__net_ts/detail/config.hpp
+++ w/include/experimental/__net_ts/detail/config.hpp
@@ -777,7 +777,9 @@
 # if !defined(NET_TS_DISABLE_STD_STRING_VIEW)
 #  if defined(__clang__)
 #   if (__cplusplus >= 201402)
-#    if __has_include(<experimental/string_view>)
+#    if __has_include(<string_view>)
+#     define NET_TS_HAS_STD_STRING_VIEW 1
+#    elif __has_include(<experimental/string_view>)
 #     define NET_TS_HAS_STD_STRING_VIEW 1
 #     define NET_TS_HAS_STD_EXPERIMENTAL_STRING_VIEW 1
 #    endif // __has_include(<experimental/string_view>)

[chriskohlhoff]

Actually, the clang compilation error is technically correct (the best kind of correct) and the MSVC compilation success is wrong. This is because the Networking TS targets C++14 and the Fundamentals TS v1, as C++17 isn't official yet.

I don't have the latest MSVC handy. Does it provide std::experimental::string_view?

I'll have a think about the best way to support both std::experimental::string_view and std::string_view simultaneously, assuming both are present.

P.S.: Is it intentional that basic_resolver_results inherits privately from basic_resolver_iterator? Should I create a second issue regarding this?

Yes, it is intentional. It's part of a mechanism for backward compatibility with asio's old resolve interface. See:

https://github.com/chriskohlhoff/asio/blob/582ed12f7c18f43826c616209cd78cfa0d9acad7/asio/include/asio/ip/basic_resolver_results.hpp#L51

You don't see this mechanism in networking-ts-impl because the tsify.pl script strips out the deprecated code.

P.P.S.: Great implementation even if it's "just" converted from ASIO. I even see a slight performance increase (~5%). Thank you!

Interesting. What is the baseline for the comparison?

[qis]

Visual Studio 2017 has no <experimental/string_view>. It has only <string_view> and the class lives in the std namespace directly. Your config.hpp sets the correct variables for it to work.

I'll publish the benchmark later.

[qis]

I take everything back about the performance boost. Here is the benchmark and some results:

https://github.com/qis/network-ts-benchmark

I would be grateful if somebody would skim over it and point out mistakes.

[chriskohlhoff]

Most likely there is noise due to other factors. E.g. assignment to different CPUs, power management state, etc. To eliminate these the benchmark would need to set affinity, disable power management in the OS and BIOS, isolate CPUs from other processes, etc.

If you can't be bothered doing all that, you could probably just assume that the performance is actually the same (as expected) and any difference is noise :)

[qis]

I'll lay it off until next weekend. Having reliable benchmarks would allow me to test and compare my own code. Thanks for the suggestion!

[cliffg-softwarelibre]

I'm using g++ 7.2 (clang and vc++ compiles will be coming next), which has full C++ 17 support for std::string_view, so I'd enjoy being able to use std::string_view with async_resolve as well. (I have the same issue with async_read_until.)

I understand the Networking TS support is C++14, but a config option supporting std::string_view would be appreciated, specially since it looks like you have most of the "plumbing" in place.