2

I test my code using travis. Recently someone added gcc9 to the set of compilers the code gets tested with. While everything compiles fine with gcc8 (both with c++14 and c++17) and gcc-9.1.0 with c++14 it fails with gcc-9.1.0 with c++17 with the following error:

/usr/include/c++/9/functional: In instantiation of ‘std::_Bind<_Functor(_Bound_args ...)>::_Bind(const _Functor&, _Args&& ...) [with _Args = {std::tuple<int>}; _Functor = SQLite::Statement; _Bound_args = {std::tuple<int>}]’:
/usr/include/c++/9/functional:811:38:   required from ‘typename std::_Bind_helper<std::__is_socketlike<_Func>::value, _Func, _BoundArgs ...>::type std::bind(_Func&&, _BoundArgs&& ...) [with _Func = SQLite::Statement&; _BoundArgs = {std::tuple<int>}; typename std::_Bind_helper<std::__is_socketlike<_Func>::value, _Func, _BoundArgs ...>::type = std::_Bind<SQLite::Statement(std::tuple<int>)>]’
/home/travis/build/SRombauts/SQLiteCpp/include/SQLiteCpp/ExecuteMany.h:84:9:   required from ‘void SQLite::bind_exec(SQLite::Statement&, std::tuple<_Tps ...>&&) [with Types = {int}]’
/home/travis/build/SRombauts/SQLiteCpp/include/SQLiteCpp/ExecuteMany.h:50:14:   required from ‘void SQLite::execute_many(SQLite::Database&, const char*, Arg&&, Types&& ...) [with Arg = std::tuple<int>; Types = {std::tuple<int, const char*>, std::tuple<int, const char*>}]’
/home/travis/build/SRombauts/SQLiteCpp/tests/ExecuteMany_test.cpp:35:9:   required from here
/usr/include/c++/9/functional:462:59: error: ‘SQLite::Statement::Statement(const SQLite::Statement&)’ is private within this context
  462 |  : _M_f(__f), _M_bound_args(std::forward<_Args>(__args)...)
      |                                                           ^
In file included from /home/travis/build/SRombauts/SQLiteCpp/include/SQLiteCpp/Column.h:13,
                 from /home/travis/build/SRombauts/SQLiteCpp/include/SQLiteCpp/Database.h:13,
                 from /home/travis/build/SRombauts/SQLiteCpp/tests/ExecuteMany_test.cpp:13:
/home/travis/build/SRombauts/SQLiteCpp/include/SQLiteCpp/Statement.h:696:5: note: declared private here
  696 |     Statement(const Statement&);

the code that throws this error is the following:

template <typename Arg, typename... Types>
void execute_many(Database& aDatabase, const char* apQuery, Arg&& aArg, Types&&... aParams)
{
    Statement query(aDatabase, apQuery);
    bind_exec(query, std::forward<Arg>(aArg));
    (void)std::initializer_list<int>
    {
        ((void)reset_bind_exec(query, std::forward<Types>(aParams)), 0)...
    };
}

template <typename TupleT>
void reset_bind_exec(Statement& apQuery, TupleT&& aTuple)
{
    apQuery.reset();
    bind_exec(apQuery, std::forward<TupleT>(aTuple));
}

template <typename TupleT>
void bind_exec(Statement& apQuery, TupleT&& aTuple)
{
    bind(apQuery, std::forward<TupleT>(aTuple));
    while (apQuery.executeStep()) {}
}

I use the following code for travis CI to use the corresponding compiler

matrix:
  include:
    - compiler: gcc
      addons:
        apt:
          sources:
            - ubuntu-toolchain-r-test
          packages:
            - g++-9
      env:
        - CC=gcc-9
        - CXX=g++-9
        - CXXFLAGS="-std=c++17 -Wall -Wextra -pedantic"

before_install:
  # coveralls test coverage:
  - if [[ "$CXX" == "g++" ]]; then pip install --user cpp-coveralls ; fi

# scripts to run before build
before_script:
  - gcc --version
  - mkdir build
  - cd build
  - cmake -DCMAKE_BUILD_TYPE=Debug -DSQLITECPP_USE_GCOV=ON -DSQLITECPP_BUILD_EXAMPLES=ON -DSQLITECPP_BUILD_TESTS=ON ..

# build examples, and run tests (ie make & make test)
script:
  - cmake --build .
  - ctest --verbose --output-on-failure

the class Statement has a private copy constructor and assignment operator, but I wonder why this would cause any issue here, because I do not copy the Statement "query". Especially why this problem only occurs with gcc-9.1.0 with c++17 (on my local machine I use gcc-9.1.1 and it compiles without any errors).

maxbachmann
  • 2,862
  • 1
  • 11
  • 35
  • 1
    Please show a [mre]. Is `bind_exec` supposed to be using `std::bind` or some other function called `bind`? If it isn't get rid of the `using namespace std` you have somewhere. If it is supposed to be `std::bind` get rid of the `using namespace std` anyway. – Alan Birtles Jun 26 '19 at 07:22
  • this is a wrapper around the sqlite3 c library. I will have to work on creating a minimal reproducible example later on , since I can't reproduce it myself (the error occurs when building with gcc 9.1.0 with travis CI, but not locally with gcc 9.1.1). (source code is at https://github.com/maxbachmann/SQLiteCpp). The confusing thing is that actually it should not require a copy constructor for the Statement class and even if it would it should fail for example in other versions of gcc aswell. Here is the corresponding travis build: https://travis-ci.org/SRombauts/SQLiteCpp/builds/549915185 – maxbachmann Jun 26 '19 at 07:32
  • A quick hack on godbolt fails on all versions of clang and gcc: https://godbolt.org/z/Qlya1l, changing `bind` to `SQLite::bind` fixes it, `std::bind` is getting picked up by ADL for some reason rather than `SQLite::bind` – Alan Birtles Jun 26 '19 at 07:37
  • wow need to try that (do you have a idea why this happens since I do not use namespace std) – maxbachmann Jun 26 '19 at 07:42
  • 1
    See https://www.modernescpp.com/index.php/c-core-guidelines-argument-dependent-lookup-or-koenig-lookup – Alan Birtles Jun 26 '19 at 07:44
  • wow thank you I was not aware of this issue. So even in namespace you should still always add the scope? – maxbachmann Jun 26 '19 at 07:51
  • its better to avoid declaring functions with the same name as standard functions – Alan Birtles Jun 26 '19 at 08:29

1 Answers1

1

A more minimal example is this:

#include <functional>

namespace SQLite
{
    struct Statement
    {
        Statement() = default;
        Statement(const Statement&) = delete;
    };
    template<class ...Args>
    void bind( Statement& s, const Args& ... args )
    {        
    }

    template < typename T >
    void test(T&& t)
    {
        Statement s;
        bind( s, std::forward< T >( t ) );
    }    
}

int main() 
{
    std::tuple< int > t;
    SQLite::test( t );
}

As one of your arguments is from the std namespace Argument Dependant Lookup brings std::bind into the list of usable functions. SQLite::bind requires a conversion to const& before calling so std::bind is a better match.

You have a few options to fix this:

  1. explicitly call SQLite::bind
  2. change the name of bind to something that isn't in the standard library (this may be the best option as it stops your users from running into the same issue)
  3. change bind's arguments from const Args& ... args to Args&& ... args to remove the conversion
Alan Birtles
  • 32,622
  • 4
  • 31
  • 60