std::string_view support

[dvtate]

My goal is to fix this issue: #517

Previous, similar PRs:

• Added some bindings for std::string_view when using c++17 #224 : ambiguous overloads
• Supports string_view for bind #355 : missing C++17 ifdef guards

I should probably add unit tests and examples using string_view.

[jagerman]

@dvtate - it would be nice to also see this solve #533 where string_view is not only nice to have but actually critically important for an application that wants to use secure memory for encrypted database keys.

[dvtate]

I just added functions to handle #533 but I'm getting a linker error with the unit tests. I need to sleep now but I can try and figure out what's going on with the unit test builds over the weekend.

[dvtate]

@jagerman , in order to maintain ABI compatibility and avoid the compiler errors that come with the ambiguity of const std::string_view vs. const std::string& (ie - implicit cast from string literal), the new key and rekey member functions should accept (const char* data, const size_t length). This also would benefit anyone who doesn't have C++17 yet. I'll adjust the code altho admittedly this might now be outside the scope of this PR.

To clarify, the linker error was caused by the library being compiled with an older C++ standard than the tests... which is something that we probably want to support.

[dvtate]

Just pushed those changes.

@SRombauts, does this look like something you think would make sense for the project? If not, please let me know and I'll close the PR.

[jagerman]

does this look like something you think would make sense for the project?

I know you didn't ask me, but personally I find this PR highly valuable: currently this library has no ability to bind TEXT values into statement placeholders without copying, which feels like a big omission versus the C API, which allows this easily.

I tried to work around the limitation by seeing if I could get the raw C API statement handle out of a SqliteCpp::Statement so that I could bind such values myself via the C API, but that proved impossible: Statement's internal handle is private and thus entirely inaccessible and so my choices were either: 1) avoid SQLite::Statement entirely and use the C API for all binding if I want to be able to do copy-less TEXT binding; 2) give up and just accept unnecessary copying; 3) fork the project to add it.

I opted for 3, but luckily found this PR already written and so am currently building on a fork with this merged.

(The key changes here are much less important, because Database does expose the C handle, so there it's pretty simple to just go to the C API for that one call).

[jagerman]

@jagerman , in order to maintain ABI compatibility and avoid the compiler errors that come with the ambiguity of const std::string_view vs. const std::string& (ie - implicit cast from string literal), the new key and rekey member functions should accept (const char* data, const size_t length). This also would benefit anyone who doesn't have C++17 yet. I'll adjust the code altho admittedly this might now be outside the scope of this PR.

I'm not sure if you would actually want to do this here or not, but one way to avoid that ambiguity is to define a C literal overload as well:

void key(const std::string& k) {(void)k;}
void key(const char* key) {(void)key;}
void key(std::string_view k) { (void) k; }

int main() {
    key("foo");
}

It is starting to feel a bit heavy though, and the pointer+size version would be perfectly fine for me.

[dvtate]

Agreed, but this library has been around for a while and idk how enthusiastic maintainers are about new features here.

I forgot that's an option. I personally like the pointer+len approach more because otherwise I'd have to convert my custom secure string type to std::string_view since it presumably could have '\0' characters... But I'd be happy to implement whatever approach makes reviewers happy.