SOCI / soci

Official repository of the SOCI - The C++ Database Access Library
http://soci.sourceforge.net/
Boost Software License 1.0
1.43k stars 479 forks source link

use_container - const std::string& #700

Open alastairUK opened 5 years ago

alastairUK commented 5 years ago

Looks like I had a coding error that up until recently was going undetected {not sure if moving to VS2017/stdcpplatest/permissive- caused this to show up - I need to do some more investigation I think}

Basically had something like this:

sql << "insert into result(id, sw_name, sw_version) values(:v1, :v2, :v3)", soci::use(data.ID()), soci::use(swName), soci::use(std::to_string(versionNum)));

the std::to_string is creating a temporary which gets bound to the const std::string& and as a result nothing gets written in that field.

As I said it had worked before - the joys of undefined behaviour!

I am aware (now) of the section in the user guide called - "Object lifetime and immutability" but still think this is a gotcha that could be possibly be resolved.

There used to be a clang-tidy check

google-runtime-member-string-references

http://releases.llvm.org/5.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/google-runtime-member-string-references.html

That warned against such a pattern

This has been removed https://github.com/llvm-mirror/clang-tools-extra/commit/0994d5f74c117f7a4417bcf91a32fa12f33b2957 but is still a valid warning I think.

I am wondering if it would be better to take the string by value and std::move into a std::string member? {assuming C++11}

Or potentially = delete an overload of the function that takes a && to stop users using the API in an unsafe manner. Note: I have tried this approach and it is showing a number of places this pattern seems to be being used which I guess is sub-optimal.

Apologies if this has been discussed before and it's simply a case that I should have RTFM properly before using the API.

mloskot commented 5 years ago

Short of time, so quick one: SOCI has not moved to C++11 yet. AFAIK/R, SOCI 4 is planned as last release based on C++03.

vadz commented 5 years ago

It would indeed be nice to remove the && overload. We could probably do it even now, inside the appropriate preprocessor check, but it's going to be a big ugly. We definitely should do it in SOCI 5, once C++11 (or maybe even C++17 by then) becomes required.