C++17 string_view

[SethHamilton]

Simple-Web-Server/client_http.hpp

Line 19 in 1056bd2

using string_view = const std::string &; // TODO c++17: use std::string_view

I changed this line to:

using string_view = std::string_view;

and life became wonderful. My application keeps data in char* format (with a known size_t). I was pushing these char* buffers into streams, and passing them to request. I realized that this results in 2 copies of the data (one to create my stream, and another request uses << to push that stream into ASIO).

string_view solves this wonderfully, as it simply wraps the pointer and length, and results in one less copy (some of my transfers are over 100mb, making copies painful and memory intensive).

I realize your original code comment might have to do with C++17 feature detection, but I think this can be cured with the following:

#ifdef __has_include 
  #if __has_include(<string_view>)
    #include<string_view>
    using string_view = std::string_view;
    #define __has_string_view 1
  #endif
#endif
#ifndef __has_string_view
  using string_view = const std::string &;
#endif

Super-ultra-nice would be versions of request that took a char* and a size_t, I could see this being a super common use case.

[eidheim]

Brilliant! Would you mind creating a pull request so that you get credited for this:)?

[SethHamilton]

Lol, sure, I'll do it for the fame and fortune!

[SethHamilton]

@eidheim Can't seem to push my PR. Getting permission denied.

[pboettch]

@SethHamilton You need to fork this repository on your account, then you push your changes there. Then you can create a Pull Request to eidheim's repo.

[SethHamilton]

I've made the PR here: #214

This is failing CI, but I think it's because it's not set to the C++17 standard for compilation.

[SethHamilton]

@pboettch @eidheim Ok, so, there is a (stupid) flaw with __has_include, it checks for the presence of the include, not whether it will compile or not. When not compiling with C++17 flags it will still pass. I'm now thinking a simple USE_CPP17_STRING_VIEW define might be the best way of selecting this.

The alternative would be to use the __cplusplus flag and check for C++17, but compilers like MSVC are refusing to switch this flag to the standard date for C++17 until they are fully compliant (gcc and clang don't seem to make this distinction). So, the option there would be to test for the right __cplusplus version if it's not MSVC and perhaps another test for a specific MSVC define (if I can find one).

[SethHamilton]

like perhaps something like this:

#if defined(__has_include) && __has_include(<string_view>) && (__cplusplus >> 201402L || (defined(_MSC_VER) && _MSC_VER >= 1912))
#include <string_view>
namespace SimpleWeb { using string_view = std::string_view; }
#elif !defined(USE_STANDALONE_ASIO)
#include <boost/utility/string_ref.hpp>
namespace SimpleWeb { using string_view = boost::string_ref; }  
#else
namespace SimpleWeb { using string_view = const std::string &; }
#endif

I'm starting to lean toward something like USE_CPP17_STRING_VIEW, but I also see that potentially limiting going forward, C++17 features like string_view are going to be normal eventually.

[SethHamilton]

This is compiling. I've added two tests with the s-std=c++17 compiler flag with and without USE_STANDALONE_ASIO
#214