Request for comment: removing std::bind
Open, Needs TriagePublic
Actions

Description

Hi,

As requested by @dkazakov in the last meeting, I'm filing this one to gauge support for a change to our code style.

The change in question I am proposing is to stop using std::bind in new code, and remove the existing (~230) instances from the codebase before they increase due to the impending Lager rewrite.

Rationale

From a general point of view, std::bind is one of the discouraged language features by clang-tidy 1:

std::bind can be hard to read and can result in larger object files and binaries due to type information that will not be produced by equivalent lambdas.

For example, when debugging the PSD parser for bug 464700, the use of bind for the XML callbacks resulted in a stacktrace like this, where the actual function that's calling the pattern finder is invisible, inside the innards of std::function:

kritaimage.dll!KisAslLayerStyleSerializer::assignPatternObject(const QString & patternUuid, const QString & patternName, std::function<void __cdecl(QSharedPointer<KoPattern>)> setPattern) Line 949 (e:\krita-win\src\libs\image\kis_asl_layer_style_serializer.cpp:949)
[Inline Frame] kritaimage.dll!std::invoke(void(KisAslLayerStyleSerializer::*)(const QString &, const QString &, std::function<void __cdecl(QSharedPointer<KoPattern>)>) &) Line 1580 (c:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32213\include\type_traits:1580)
[Inline Frame] kritaimage.dll!std::_Invoker_ret<std::_Unforced>::_Call(void(KisAslLayerStyleSerializer::*)(const QString &, const QString &, std::function<void __cdecl(QSharedPointer<KoPattern>)>) &) Line 683 (c:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32213\include\functional:683)
[Inline Frame] kritaimage.dll!std::_Call_binder(std::_Invoker_ret<std::_Unforced>) Line 1975 (c:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32213\include\functional:1975)
[Inline Frame] kritaimage.dll!std::_Binder<std::_Unforced,void (__cdecl KisAslLayerStyleSerializer::*)(QString const &,QString const &,std::function<void __cdecl(QSharedPointer<KoPattern>)>),KisAslLayerStyleSerializer *,std::_Ph<1> const &,std::_Ph<2> const &,std::function<void __cdecl(QSharedPointer<KoPattern>)> &>::operator()(const QString &) Line 2012 (c:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32213\include\functional:2012)
[Inline Frame] kritaimage.dll!std::invoke(std::_Binder<std::_Unforced,void (__cdecl KisAslLayerStyleSerializer::*)(QString const &,QString const &,std::function<void __cdecl(QSharedPointer<KoPattern>)>),KisAslLayerStyleSerializer *,std::_Ph<1> const &,std::_Ph<2> const &,std::function<void __cdecl(QSharedPointer<KoPattern>)> &> &) Line 1574 (c:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32213\include\type_traits:1574)
[Inline Frame] kritaimage.dll!std::_Invoker_ret<void>::_Call(std::_Binder<std::_Unforced,void (__cdecl KisAslLayerStyleSerializer::*)(QString const &,QString const &,std::function<void __cdecl(QSharedPointer<KoPattern>)>),KisAslLayerStyleSerializer *,std::_Ph<1> const &,std::_Ph<2> const &,std::function<void __cdecl(QSharedPointer<KoPattern>)> &> &) Line 670 (c:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32213\include\functional:670)
kritaimage.dll!std::_Func_impl_no_alloc<std::_Binder<std::_Unforced,void (__cdecl KisAslLayerStyleSerializer::*)(QString const &,QString const &,std::function<void __cdecl(QSharedPointer<KoPattern>)>),KisAslLayerStyleSerializer *,std::_Ph<1> const &,std::_Ph<2> const &,std::function<void __cdecl(QSharedPointer<KoPattern>)> &>,void,QString const &,QString const &>::_Do_call(const QString & <_Args_0>, const QString & <_Args_1>) Line 834 (c:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32213\include\functional:834)
[Inline Frame] kritapsdutils.dll!std::_Func_class<void,QString const &,QString const &>::operator()(const QString &) Line 874 (c:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.35.32213\include\functional:874)
kritapsdutils.dll!KisAslCallbackObjectCatcher::addPatternRef(const QString & path, const QString & patternUuid, const QString & patternName) Line 178 (e:\krita-win\src\libs\psdutils\asl\kis_asl_callback_object_catcher.cpp:178)

Scott Meyers in his book "Effective Modern C++" (see item 34) has also recommended its outright removal from C++14 onwards:

This history means that some programmers have a decade or more of experience using std::bind. If you’re one of them, you may be reluctant to abandon a tool that’s served you well. That’s understandable, but in this case, change is good, because in C++11, lambdas are almost always a better choice than std::bind. As of C++14, the case for lambdas isn’t just stronger, it’s downright ironclad.

He identifies three issues with std::bind:

inability to automatically choose overloads based on function parameter types
inability to go beyond simple parameter type binding
the parameter passing is by value, never by reference or capture
the parameter placeholding is completely opaque to the developer

Uses

We use std::bind in three places:

the vast majority (134 through macro, 41 directly) as callbacks for the ASL parser
9 as callbacks for KisSignalCompressorWithParam
the remaining as either isolated instances of std::function or a way to implicitly cast values between types

lsegovia created this task.Jan 28 2023, 5:24 PM

So, std::bind is to me largely an incantation that happens within the PSD parsing code. What would we replace it with?

EDIT: Oh, the quote suggests lambdas :)

I'm not a fan of std::bind, but we need to be pretty careful with lambdas, they're often a source of bugs, too.

Hi, @lsegovia!

I have seen this warning when writing the lager code. I even tried to switch to the lambda syntax, but the resulting code looks a bit too verbose. Specifically, you need to explicitly declare all the typenames for every function argument (which might not known in the template context).

In lager branch I used bind a lot to do lager-specific signal connections, so it is used a lot for trivial things, like "binding model values to real widgets".

Here is an example from KisCurveOptionInputControlsStrategy:

Code with std::bind:

xValueSuffix.bind(std::bind(&SpinBox::setSuffix, inSpinBox, std::placeholders::_1));
yValueSuffix.bind(std::bind(&SpinBox::setSuffix, outSpinBox, std::placeholders::_1));
rangeValues.bind(
            kismpl::unzip_wrapper(std::bind(&KisCurveOptionInputControlsStrategy::updateCurveLabels, this,
                                                 std::placeholders::_1, std::placeholders::_2,
                                                 std::placeholders::_3, std::placeholders::_4)));

Code with lambdas:

xValueSuffix.bind([inSpinBox = inSpinBox] (const QString &suffix) { inSpinBox->setSuffix(suffix); });
yValueSuffix.bind([outSpinBox = outSpinBox] (const QString &suffix) { outSpinBox->setSuffix(suffix); });

rangeValues.bind(
    kismpl::unzip_wrapper([this] (qreal xMin, qreal xMax, qreal yMin, qreal yMax) {
        this->updateCurveLabels(xMin, xMax, yMin, yMax);
    }));

I feel like the version with lambdas is a bit too verbose, especially in the trivial case for xValueSuffix and yValueSuffix. One has to write a lot of boilerplate code which is not too relevant to the problem area.

About the length of the error messages, I'm not sure lambdas make them much shorter and/or understandable. Here is an example of incorrect number of arguments passed through the signal:

std::bind case:

In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:7:
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.h:12:
In file included from C:/dev/env-1/i/include/lager/reader.hpp:19:
C:/dev/env-1/i/include/lager/watch.hpp:98:9: error: no matching function for call to object of type 'std::__bind<void (QSpinBox::*)(const QString &), QSpinBox *&, const std::placeholders::__ph<1> &, int>'
        callback(node()->last());
        ^~~~~~~~
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:63:18: note: in instantiation of function template specialization 'lager::watchable_base<lager::detail::reader_node<QString>>::bind<std::__bind<void (QSpinBox::*)(const QString &), QSpinBox *&, const std::placeholders::__ph<1> &, int>>' requested here
    xValueSuffix.bind(std::bind(&SpinBox::setSuffix, inSpinBox, std::placeholders::_1, 0));
                 ^
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:115:40: note: in instantiation of member function 'KisCurveOptionInputControlsStrategy<QSpinBox>::KisCurveOptionInputControlsStrategy' requested here
template class PAINTOP_EXPORT_INSTANCE KisCurveOptionInputControlsStrategy<QSpinBox>;
                                       ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__functional/bind.h:293:9: note: candidate template ignored: substitution failure [with _Args = <const QString &>]: implicit instantiation of undefined template 'std::__bind_return<void (QSpinBox::*)(const QString &), std::tuple<QSpinBox *, std::placeholders::__ph<1>, int>, std::tuple<const QString &>, false>'
        operator()(_Args&& ...__args)
        ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__functional/bind.h:302:9: note: candidate template ignored: substitution failure [with _Args = <const QString &>]: implicit instantiation of undefined template 'std::__bind_return<void (QSpinBox::*const)(const QString &), const std::tuple<QSpinBox *, std::placeholders::__ph<1>, int>, std::tuple<const QString &>, false>'
        operator()(_Args&& ...__args) const
        ^
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:7:
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.h:12:
In file included from C:/dev/env-1/i/include/lager/reader.hpp:19:
C:/dev/env-1/i/include/lager/watch.hpp:98:9: error: no matching function for call to object of type 'std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>'
        callback(node()->last());
        ^~~~~~~~
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:63:18: note: in instantiation of function template specialization 'lager::watchable_base<lager::detail::reader_node<QString>>::bind<std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>>' requested here
    xValueSuffix.bind(std::bind(&SpinBox::setSuffix, inSpinBox, std::placeholders::_1, 0));
                 ^
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:116:40: note: in instantiation of member function 'KisCurveOptionInputControlsStrategy<QDoubleSpinBox>::KisCurveOptionInputControlsStrategy' requested here
template class PAINTOP_EXPORT_INSTANCE KisCurveOptionInputControlsStrategy<QDoubleSpinBox>;
                                       ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__functional/bind.h:293:9: note: candidate template ignored: substitution failure [with _Args = <const QString &>]: implicit instantiation of undefined template 'std::__bind_return<void (QDoubleSpinBox::*)(const QString &), std::tuple<QDoubleSpinBox *, std::placeholders::__ph<1>, int>, std::tuple<const QString &>, false>'
        operator()(_Args&& ...__args)
        ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__functional/bind.h:302:9: note: candidate template ignored: substitution failure [with _Args = <const QString &>]: implicit instantiation of undefined template 'std::__bind_return<void (QDoubleSpinBox::*const)(const QString &), const std::tuple<QDoubleSpinBox *, std::placeholders::__ph<1>, int>, std::tuple<const QString &>, false>'
        operator()(_Args&& ...__args) const
        ^
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:7:
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.h:12:
In file included from C:/dev/env-1/i/include/lager/reader.hpp:16:
In file included from C:/dev/env-1/i/include/lager/detail/nodes.hpp:41:
C:/dev/env-1/i/include/lager/detail/signal.hpp:49:47: error: no matching function for call to object of type 'std::__bind<void (QSpinBox::*)(const QString &), QSpinBox *&, const std::placeholders::__ph<1> &, int>'
        void operator()(Args... args) final { fn_(args...); }
                                              ^~~
C:/dev/env-1/i/include/lager/detail/signal.hpp:46:9: note: in instantiation of member function 'lager::detail::signal<const QString &>::slot<std::__bind<void (QSpinBox::*)(const QString &), QSpinBox *&, const std::placeholders::__ph<1> &, int>>::operator()' requested here
        slot(Fn fn)
        ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__memory/unique_ptr.h:714:32: note: in instantiation of member function 'lager::detail::signal<const QString &>::slot<std::__bind<void (QSpinBox::*)(const QString &), QSpinBox *&, const std::placeholders::__ph<1> &, int>>::slot' requested here
    return unique_ptr<_Tp>(new _Tp(_VSTD::forward<_Args>(__args)...));
                               ^
C:/dev/env-1/i/include/lager/detail/signal.hpp:66:29: note: in instantiation of function template specialization 'std::make_unique<lager::detail::signal<const QString &>::slot<std::__bind<void (QSpinBox::*)(const QString &), QSpinBox *&, const std::placeholders::__ph<1> &, int>>, std::__bind<void (QSpinBox::*)(const QString &), QSpinBox *&, const std::placeholders::__ph<1> &, int>>' requested here
        auto s       = std::make_unique<slot_t>(std::forward<Fn>(fn));
                            ^
C:/dev/env-1/i/include/lager/watch.hpp:91:34: note: in instantiation of function template specialization 'lager::detail::signal<const QString &>::connect<std::__bind<void (QSpinBox::*)(const QString &), QSpinBox *&, const std::placeholders::__ph<1> &, int>>' requested here
        conns_.push_back(base_t::connect(std::forward<CallbackT>(callback)));
                                 ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__functional/bind.h:293:9: note: candidate template ignored: substitution failure [with _Args = <const QString &>]: implicit instantiation of undefined template 'std::__bind_return<void (QSpinBox::*)(const QString &), std::tuple<QSpinBox *, std::placeholders::__ph<1>, int>, std::tuple<const QString &>, false>'
        operator()(_Args&& ...__args)
        ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__functional/bind.h:302:9: note: candidate template ignored: substitution failure [with _Args = <const QString &>]: implicit instantiation of undefined template 'std::__bind_return<void (QSpinBox::*const)(const QString &), const std::tuple<QSpinBox *, std::placeholders::__ph<1>, int>, std::tuple<const QString &>, false>'
        operator()(_Args&& ...__args) const
        ^
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:7:
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.h:12:
In file included from C:/dev/env-1/i/include/lager/reader.hpp:16:
In file included from C:/dev/env-1/i/include/lager/detail/nodes.hpp:41:
C:/dev/env-1/i/include/lager/detail/signal.hpp:49:47: error: no matching function for call to object of type 'std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>'
        void operator()(Args... args) final { fn_(args...); }
                                              ^~~
C:/dev/env-1/i/include/lager/detail/signal.hpp:46:9: note: in instantiation of member function 'lager::detail::signal<const QString &>::slot<std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>>::operator()' requested here
        slot(Fn fn)
        ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__memory/unique_ptr.h:714:32: note: in instantiation of member function 'lager::detail::signal<const QString &>::slot<std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>>::slot' requested here
    return unique_ptr<_Tp>(new _Tp(_VSTD::forward<_Args>(__args)...));
                               ^
C:/dev/env-1/i/include/lager/detail/signal.hpp:66:29: note: in instantiation of function template specialization 'std::make_unique<lager::detail::signal<const QString &>::slot<std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>>, std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>>' requested here
        auto s       = std::make_unique<slot_t>(std::forward<Fn>(fn));
                            ^
C:/dev/env-1/i/include/lager/watch.hpp:91:34: note: in instantiation of function template specialization 'lager::detail::signal<const QString &>::connect<std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>>' requested here
        conns_.push_back(base_t::connect(std::forward<CallbackT>(callback)));
                                 ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__functional/bind.h:293:9: note: candidate template ignored: substitution failure [with _Args = <const QString &>]: implicit instantiation of undefined template 'std::__bind_return<void (QDoubleSpinBox::*)(const QString &), std::tuple<QDoubleSpinBox *, std::placeholders::__ph<1>, int>, std::tuple<const QString &>, false>'
        operator()(_Args&& ...__args)
        ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__functional/bind.h:302:9: note: candidate template ignored: substitution failure [with _Args = <const QString &>]: implicit instantiation of undefined template 'std::__bind_return<void (QDoubleSpinBox::*const)(const QString &), const std::tuple<QDoubleSpinBox *, std::placeholders::__ph<1>, int>, std::tuple<const QString &>, false>'
        operator()(_Args&& ...__args) const
        ^
4 errors generated.

Lambda case:

In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:7:
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.h:12:
In file included from C:/dev/env-1/i/include/lager/reader.hpp:19:
C:/dev/env-1/i/include/lager/watch.hpp:98:9: error: no matching function for call to object of type '(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)'
        callback(node()->last());
        ^~~~~~~~
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:18: note: in instantiation of function template specialization 'lager::watchable_base<lager::detail::reader_node<QString>>::bind<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>' requested here
    xValueSuffix.bind([inSpinBox = inSpinBox] (const QString &suffix, int i) { inSpinBox->setSuffix(suffix); });
                 ^
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:115:40: note: in instantiation of member function 'KisCurveOptionInputControlsStrategy<QSpinBox>::KisCurveOptionInputControlsStrategy' requested here
template class PAINTOP_EXPORT_INSTANCE KisCurveOptionInputControlsStrategy<QSpinBox>;
                                       ^
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23: note: candidate function not viable: requires 2 arguments, but 1 was provided
    xValueSuffix.bind([inSpinBox = inSpinBox] (const QString &suffix, int i) { inSpinBox->setSuffix(suffix); });
                      ^
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:7:
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.h:12:
In file included from C:/dev/env-1/i/include/lager/reader.hpp:19:
C:/dev/env-1/i/include/lager/watch.hpp:98:9: error: no matching function for call to object of type '(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)'
        callback(node()->last());
        ^~~~~~~~
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:18: note: in instantiation of function template specialization 'lager::watchable_base<lager::detail::reader_node<QString>>::bind<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>' requested here
    xValueSuffix.bind([inSpinBox = inSpinBox] (const QString &suffix, int i) { inSpinBox->setSuffix(suffix); });
                 ^
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:116:40: note: in instantiation of member function 'KisCurveOptionInputControlsStrategy<QDoubleSpinBox>::KisCurveOptionInputControlsStrategy' requested here
template class PAINTOP_EXPORT_INSTANCE KisCurveOptionInputControlsStrategy<QDoubleSpinBox>;
                                       ^
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23: note: candidate function not viable: requires 2 arguments, but 1 was provided
    xValueSuffix.bind([inSpinBox = inSpinBox] (const QString &suffix, int i) { inSpinBox->setSuffix(suffix); });
                      ^
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:7:
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.h:12:
In file included from C:/dev/env-1/i/include/lager/reader.hpp:16:
In file included from C:/dev/env-1/i/include/lager/detail/nodes.hpp:41:
C:/dev/env-1/i/include/lager/detail/signal.hpp:49:47: error: no matching function for call to object of type '(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)'
        void operator()(Args... args) final { fn_(args...); }
                                              ^~~
C:/dev/env-1/i/include/lager/detail/signal.hpp:46:9: note: in instantiation of member function 'lager::detail::signal<const QString &>::slot<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>::operator()' requested here
        slot(Fn fn)
        ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__memory/unique_ptr.h:714:32: note: in instantiation of member function 'lager::detail::signal<const QString &>::slot<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>::slot' requested here
    return unique_ptr<_Tp>(new _Tp(_VSTD::forward<_Args>(__args)...));
                               ^
C:/dev/env-1/i/include/lager/detail/signal.hpp:66:29: note: in instantiation of function template specialization 'std::make_unique<lager::detail::signal<const QString &>::slot<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>, (lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>' requested here
        auto s       = std::make_unique<slot_t>(std::forward<Fn>(fn));
                            ^
C:/dev/env-1/i/include/lager/watch.hpp:91:34: note: in instantiation of function template specialization 'lager::detail::signal<const QString &>::connect<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>' requested here
        conns_.push_back(base_t::connect(std::forward<CallbackT>(callback)));
                                 ^
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23: note: candidate function not viable: requires 2 arguments, but 1 was provided
    xValueSuffix.bind([inSpinBox = inSpinBox] (const QString &suffix, int i) { inSpinBox->setSuffix(suffix); });
                      ^
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:7:
In file included from C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.h:12:
In file included from C:/dev/env-1/i/include/lager/reader.hpp:16:
In file included from C:/dev/env-1/i/include/lager/detail/nodes.hpp:41:
C:/dev/env-1/i/include/lager/detail/signal.hpp:49:47: error: no matching function for call to object of type '(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)'
        void operator()(Args... args) final { fn_(args...); }
                                              ^~~
C:/dev/env-1/i/include/lager/detail/signal.hpp:46:9: note: in instantiation of member function 'lager::detail::signal<const QString &>::slot<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>::operator()' requested here
        slot(Fn fn)
        ^
C:/deps/llvm-mingw-20220906-ucrt-x86_64/include/c++/v1/__memory/unique_ptr.h:714:32: note: in instantiation of member function 'lager::detail::signal<const QString &>::slot<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>::slot' requested here
    return unique_ptr<_Tp>(new _Tp(_VSTD::forward<_Args>(__args)...));
                               ^
C:/dev/env-1/i/include/lager/detail/signal.hpp:66:29: note: in instantiation of function template specialization 'std::make_unique<lager::detail::signal<const QString &>::slot<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>, (lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>' requested here
        auto s       = std::make_unique<slot_t>(std::forward<Fn>(fn));
                            ^
C:/dev/env-1/i/include/lager/watch.hpp:91:34: note: in instantiation of function template specialization 'lager::detail::signal<const QString &>::connect<(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)>' requested here
        conns_.push_back(base_t::connect(std::forward<CallbackT>(callback)));
                                 ^
C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23: note: candidate function not viable: requires 2 arguments, but 1 was provided
    xValueSuffix.bind([inSpinBox = inSpinBox] (const QString &suffix, int i) { inSpinBox->setSuffix(suffix); });
                      ^
1 warning and 4 errors generated.

In both the cases, QtCreator collapses the error message to one line:

std::bind case:

error: no matching function for call to object of type 'std::__bind<void (QDoubleSpinBox::*)(const QString &), QDoubleSpinBox *&, const std::placeholders::__ph<1> &, int>'

Lambda case:

error: no matching function for call to object of type '(lambda at C:/dev/env-1/krita/plugins/paintops/libpaintop/KisCurveOptionInputControlsStrategy.cpp:70:23)'

As you can see, the error message for the std::bind message is more informative, because it lists all the expected arguments of the object.

Conclusion

Personally, I still stand on my opinion that we should not blindly follow all clang-tidy (and, probably, core guidelines) rules. Some of them are very theoretical and apply really badly in real life [0], especially in old projects with large codebase.

But if someone else thinks that we should remove std::bind and has some more arguments, please tell. I'm open to discussion, perhaps we will find some solution.

[0] - e.g. modernize-pass-by-value is good for std objects, but makes code less efficient for Qt objects with lazy copying.

PS:
I'm also not very happy that we spend so much time to trying to solve imaginary bugs from clang-tidy instead of fixing real bugs from bugzilla. Yes, we should modernize our codebase, but we should somehow limit the amount of time spent on that. I have a feeling that for the last two months of my work I have spent at least 30% of my patch-review time on mere arguments about code style and clang-tidy issues. And 20% of the coding time on resolving merge conflicts caused by "clang-tidy" modernization patches. That is too much. Perhaps we should just plan some limited timeframe when all of us work on modernization? E.g. some modernization sprint or something like that?

Request for comment: removing std::bindOpen, Needs TriagePublicActions

Description

Rationale

Uses

Conclusion

Request for comment: removing std::bind
Open, Needs TriagePublic
Actions