Open SethHamilton opened 6 years ago
Brilliant! Would you mind creating a pull request so that you get credited for this:)?
Lol, sure, I'll do it for the fame and fortune!
@eidheim Can't seem to push my PR. Getting permission denied.
@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.
I've made the PR here: https://github.com/eidheim/Simple-Web-Server/pull/214
This is failing CI, but I think it's because it's not set to the C++17 standard for compilation.
@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).
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.
This is compiling. I've added two tests with the s-std=c++17
compiler flag with and without USE_STANDALONE_ASIO
https://github.com/eidheim/Simple-Web-Server/pull/214
https://github.com/eidheim/Simple-Web-Server/blob/1056bd2e70c6abdd6a334d57247f1ea18bab45ef/client_http.hpp#L19
I changed this line to:
and life became wonderful. My application keeps data in
char*
format (with a knownsize_t
). I was pushing thesechar*
buffers into streams, and passing them torequest
. I realized that this results in 2 copies of the data (one to create my stream, and anotherrequest
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:
Super-ultra-nice would be versions of request that took a
char*
and asize_t
, I could see this being a super common use case.