27

I'm trying to implement a class which wraps an arbitrary type and a mutex. To access the wrapped data, one needs to pass a function object as parameter of the locked method. The wrapper class will then pass the wrapped data as parameter to this function object.

I'd like my wrapper class to work with const & non-const, so I tried the following

#include <mutex>
#include <string>

template<typename T, typename Mutex = std::mutex>
class   Mutexed
{
private:
    T m_data;
    mutable Mutex m_mutex;

public:
    using type = T;
    using mutex_type = Mutex;

public:
    explicit Mutexed() = default;

    template<typename... Args>
    explicit Mutexed(Args&&... args)
        : m_data{std::forward<Args>(args)...}
    {}

    template<typename F>
    auto locked(F&& f) -> decltype(std::forward<F>(f)(m_data)) {
        std::lock_guard<Mutex> lock(m_mutex);
        return std::forward<F>(f)(m_data);
    }

    template<typename F>
    auto locked(F&& f) const -> decltype(std::forward<F>(f)(m_data)) {
        std::lock_guard<Mutex> lock(m_mutex);
        return std::forward<F>(f)(m_data);
    }
};

int main()
{
    Mutexed<std::string> str{"Foo"};

    str.locked([](auto &s) { /* this doesn't compile */
        s = "Bar";
    });

    str.locked([](std::string& s) { /* this compiles fine */
        s = "Baz";
    });
    return 0;
}

The first locked call with the generic lambda fails to compile with the following error

/home/foo/tests/lamdba_auto_const/lambda_auto_const/main.cpp: In instantiation of ‘main()::<lambda(auto:1&)> [with auto:1 = const std::__cxx11::basic_string<char>]’:
/home/foo/tests/lamdba_auto_const/lambda_auto_const/main.cpp:30:60:   required by substitution of ‘template<class F> decltype (forward<F>(f)(((const Mutexed<T, Mutex>*)this)->Mutexed<T, Mutex>::m_data)) Mutexed<T, Mutex>::locked(F&&) const [with F = main()::<lambda(auto:1&)>]’
/home/foo/tests/lamdba_auto_const/lambda_auto_const/main.cpp:42:6:   required from here
/home/foo/tests/lamdba_auto_const/lambda_auto_const/main.cpp:41:11: error: passing ‘const std::__cxx11::basic_string<char>’ as ‘this’ argument discards qualifiers [-fpermissive]
         s = "Bar";
           ^
In file included from /usr/include/c++/5/string:52:0,
                 from /usr/include/c++/5/stdexcept:39,
                 from /usr/include/c++/5/array:38,
                 from /usr/include/c++/5/tuple:39,
                 from /usr/include/c++/5/mutex:38,
                 from /home/foo/tests/lamdba_auto_const/lambda_auto_const/main.cpp:1:
/usr/include/c++/5/bits/basic_string.h:558:7: note:   in call to ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>& std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::operator=(const _CharT*) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’
       operator=(const _CharT* __s)
       ^

But the second call with the std::string& parameter is fine.

Why is that ? And is there a way to make it work as expected while using a generic lambda ?

Unda
  • 1,827
  • 3
  • 23
  • 35
  • 1
    @YSC A lambda, generic or otherwise, is a class, not a class template. `F` is happily resolved to that class. The fact that the class has member templates is irrelevant at that point. – Igor Tandetnik Mar 01 '19 at 15:31

1 Answers1

34

This is a problem fundamentally with what happens with SFINAE-unfriendly callables. For more reference, check out P0826.

The problem is, when you call this:

 str.locked([](auto &s) { s = "Bar"; });

We have two overloads of locked and we have to try both. The non-const overload works fine. But the const one – even if it won't be selected by overload resolution anyway – still has to be instantiated (it's a generic lambda, so to figure out what decltype(std::forward<F>(f)(m_data)) might be you have to instantiate it) and that instantiation fails within the body of the lambda. The body is outside of the immediate context, so it's not a substitution failure – it's a hard error.

When you call this:

str.locked([](std::string& s) { s = "Bar"; });

We don't need to look at the body at all during the whole process of overload resolution – we can simply reject at the call site (since you can't pass a const string into a string&).

There's not really a solution to this problem in the language today – you basically have to add constraints on your lambda to ensure that the instantiation failure happens in the immediate context of substitution rather than in the body. Something like:

str.locked([](auto &s) -> void {
    s = "Bar";
});

Note that we don't need to make this SFINAE-friendly - we just need to ensure that we can determine the return type without instantiating the body.


A more thorough language solution would have been to allow for "Deducing this" (see the section in the paper about this specific problem). But that won't be in C++20.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • 7
    Well this is a shame. +1 as this had me really scratching my head. I couldn't figure out why it was calling the const version, and as you point out it really isn't, it just has to check, and the check results in a hard error. – NathanOliver Mar 01 '19 at 15:40
  • 1
    Thank you for this explanation. Could you elaborate on "_The body is outside of the immediate context"_ a bit though? (I'm unclear of what exactly is the immediate context) – YSC Mar 01 '19 at 15:41
  • 1
    Thank you for the explanation and the workaround. It works, but since it's more verbose than writing the type, I'll do that instead of using `auto` for this case. I'll check out the paper too. – Unda Mar 01 '19 at 15:45
  • 1
    Would it work if you took the parameter as an `auto&&` parameter (though then you can pass rvalues, which you don't really want to allow)? – Nicol Bolas Mar 01 '19 at 18:03
  • @NicolBolas No, same problem. You have to structure the lambda in such a way that the overload gets rejected while instantiating the call operator, not the body. – Barry Mar 01 '19 at 19:04
  • @Barry, I can see in the beginning of your paper that this case can be solved with a friended-templated-non-member function. – Unda Mar 01 '19 at 21:36
  • 2
    @Unda Yeah, if you want to call it like `locked(str, []{...})` instead. Just changes the syntax. – Barry Mar 01 '19 at 22:00
  • @Barry, I think it's a great alternative until P0826 makes it to the standard: it works with `auto&` as one could expect and it avoids code duplication. – Unda Mar 01 '19 at 22:07
  • @Unda You mean P0847? – Barry Mar 01 '19 at 22:11
  • @Barry, Right, I thought you mentioned the same paper twice in your answer, my mistake. – Unda Mar 01 '19 at 22:18
  • 1
    The reason why we have to instantiate the body of the lambda at O/R time is because of the deduced return type. Explicitly specifying `-> void` on the lambda is sufficient; there's no need to make it fully SFINAE-correct. – T.C. Mar 02 '19 at 21:30
  • @T.C. Right you are - was conflating two entirely unrelated things. – Barry Mar 02 '19 at 22:22
  • @T.C. Why does it instantiate the lambda even though the return type doesn't depend on the lambda parameter type at all? It's always void. – Johannes Schaub - litb Jun 15 '19 at 07:21
  • @Johannes You only know it's always void by instantiating it. Unless you're suggesting special casing lexically having no `return` statements, which is pretty specific. – Barry Jun 15 '19 at 15:36
  • @Barry compilers do a lot of syntactic special casings in templates to avoid users the burden of writing `typename`. Why not do it for return-types? If in the lambda we write `return e`, and whether it's an `int e;` previously declared or an `void()`-expression, then the compiler can mark the template's return type as non-dependent and deduce it right away to that type. – Johannes Schaub - litb Jun 16 '19 at 16:17