Use std::string_view as argument instead of std::string on C++17 Compilers

[jimmyjxiao]

The major benefit is if someone tries to bind a char* parameter, we can avoid an extra copy that would happen if a new std::string was created from it. This pull request also includes some improvements for building/running the tests on MSVC, mostly because I suck at git and didn't know how to separate them into a separate pull request.

[zauguin]

Thanks for the pull request!

While this works great for users passing literals and std::strings, some changes are needed to also support not null-terminated string_views. Details inline.

[zauguin]

Nice. Updating the ReadMe and adding a test would be great. There are some non-functional changes left but these are details.

It is quite sad that this doesn't really work out for function and database names. These should mostly be covered by small string optimization anyway but we could overload them for C-strings to avoid temporary strings at least when a literal is given? After merging this we might want a separate Pull Request in that direction.

[jimmyjxiao]

While we're on the topic of avoiding extra copies, is SQLITE_TRANSIENT really necessary over SQLITE_STATIC?

[zauguin]

If we know we have a literal string we could use SQLITE_STATIC but when the user gives us a std::string(_view), we can not know how long it is valid. For example using the iterator interface or prepared statements, passed temporarys will very often be destroyed before the statement is complete. We could document the requirements and provide optional overloads for SQLITE_STATIC but I think the reduced safety is not worth the probably rather small performance advantage. Especially because I do not hink there is a way to detect errors here without something like valgrind and in most cases the database disk write will be significantly more expensive than the string copy anyway.

Of course, I would be happy to hear about ideas how this could work.

[jimmyjxiao]

Alright, Readme and tests have been updated, can we take another look at merging? Also for the SQLITE_STATIC thing, take a look here: https://github.com/zowpowow/sqlite_modern_cpp/commit/5d5ce076f28b5f58198b325026d63fb79d6f51d6 on how I think it could be implemented

[aminroosta]

@zowpowow About SQLITE_STATIC, @5d5ce07 looks promising. However i'm more worried about the interface, instead of having

db << "select * where name = ?" << binding_mode::static << "tom";

I'd recommend

db << "select * where name = ?" << sqlite::as_static("tom");

Please see #122 , we have a discussion on supporting blob types without vector<T>, The idea was the the same to have << sqlite::blob(void*, size_t) overloaded.