Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supports string_view for bind #355

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

xis19
Copy link

@xis19 xis19 commented May 24, 2022

With this patch, it is possible to bind string_view, which addresses PR #298.

@xis19
Copy link
Author

xis19 commented May 24, 2022

@SRombauts can you please help let me know how to test the code using Google Test? I am having issues compiling the test even without my patch with errors:

/Users/xiaoge_su/Projects/SQLiteCpp/tests/Statement_test.cpp:378:16: error: call to member function 'bind' is ambiguous
        insert.bind(3, integer);
        ~~~~~~~^~~~
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:131:10: note: candidate function
    void bind(const int aIndex, const int32_t       aValue);
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:135:10: note: candidate function
    void bind(const int aIndex, const uint32_t      aValue);
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:139:10: note: candidate function
    void bind(const int aIndex, const int64_t       aValue);
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:143:10: note: candidate function
    void bind(const int aIndex, const double        aValue);
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/tests/Statement_test.cpp:500:16: error: call to member function 'bind' is ambiguous
        insert.bind("@long",     integer);
        ~~~~~~~^~~~
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:208:10: note: candidate function
    void bind(const char* apName, const int32_t         aValue)
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:215:10: note: candidate function
    void bind(const char* apName, const uint32_t        aValue)
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:222:10: note: candidate function
    void bind(const char* apName, const int64_t         aValue)
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:229:10: note: candidate function
    void bind(const char* apName, const double          aValue)
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/tests/Statement_test.cpp:613:16: error: call to member function 'bind' is ambiguous
        insert.bind(along, integer);
        ~~~~~~~^~~~
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:325:10: note: candidate function
    void bind(const std::string& aName, const int32_t         aValue)
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:332:10: note: candidate function
    void bind(const std::string& aName, const uint32_t        aValue)
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:339:10: note: candidate function
    void bind(const std::string& aName, const int64_t         aValue)
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:346:10: note: candidate function
    void bind(const std::string& aName, const double          aValue)
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/tests/Statement_test.cpp:1022:11: error: call to member function 'bind' is ambiguous
    query.bind(1, 4294967297L);
    ~~~~~~^~~~
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:131:10: note: candidate function
    void bind(const int aIndex, const int32_t       aValue);
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:135:10: note: candidate function
    void bind(const int aIndex, const uint32_t      aValue);
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:139:10: note: candidate function
    void bind(const int aIndex, const int64_t       aValue);
         ^
/Users/xiaoge_su/Projects/SQLiteCpp/include/SQLiteCpp/Statement.h:143:10: note: candidate function
    void bind(const int aIndex, const double        aValue);

I am using Apple Clang:

Apple clang version 12.0.5 (clang-1205.0.22.9)
Target: x86_64-apple-darwin21.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

@Kacperos155
Copy link
Contributor

C++11 is a minimal standard version for this project.

This isn't compiling because your new methods variants with std::string_view aren't framed within C++17 requirement macro.

Thanks for trying implementing methods with std::string_view, it would be very helpful for me and probably many others. 😸

@Kacperos155
Copy link
Contributor

You should probably look at PR #224, which tried to implement this before.

@UnixY2K
Copy link
Contributor

UnixY2K commented Dec 17, 2022

Hi, with the removal of the remaining long api´s you should not see those warnings #364
this was caused due to ambiguity with some types, however now they are using the fixed length types

@UnixY2K
Copy link
Contributor

UnixY2K commented Dec 17, 2022

also keep in mind as your PR may be outdated you need to sync it with the upstream in order to receive those updates.

as for the std::string_view code please make a check if there is support for std::string_view as the minimum requirement is c++11 which does not include it, you can do something like #388 with the corrections of #393

so basically what you need to do is:

  • sync your fork with upstream so it get the fixes of the ambiguous calls
  • make a definition/macro so this feature can be disabled based on if is supported or disabled manually
  • use the macro that you defined (can be SQLITECPP_HAVE_STD_STRING_VIEW) on the new functions that use std::string_view so they only get defined/compiled when they are supported

optionally you can add the switch on CMake or Meson so any other user can disable it, if not I can add it for you on other PR.

feel free to let us know if there is something else that you may need help as this is a good addition to the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants