2

Here is an simplified version of the problem from ported from large code base. I've solved the issue, but I don't like the way I solved it.

The problematic code that doesn't compile is this I'm starting with:

#include <iostream>
#include <cstdlib>
#include <vector>
#include <cassert>
#include <algorithm>
#include <cmath>
#include <array>
#include <utility>
#include <set>
#include <functional>

class S {
  public:
    int a;
    int b;
    mutable int c;
    void set_c() { c = 222; }
};

struct cmp
{
  bool operator()(const S& lhs, const S& rhs) const
  {
    return !(lhs.a == rhs.a && lhs.b == rhs.b);
  }
};

class core {
    public: 
      std::set<S, cmp> set_of_S;
      std::function<void()> f;
    
      void set_fn() {
        f = [this]() {
            auto it = set_of_S.begin();
            it->set_c();
        };
      }
};

int main()
{
    core core;
    
    S a {.a = 2, .b = 3, .c = 0};
    S b {.a = 2, .b = 3, .c = 0};
    S c {.a = 2, .b = 4, .c = 0};
    
    core.set_of_S.insert(a);
    core.set_of_S.insert(b);
    core.set_of_S.insert(c);

    core.set_fn();
    core.f();
    
    std::cout << core.set_of_S.size() << '\n';
}

The compiler error is:

prog.cc: In lambda function:
prog.cc:37:23: error: passing 'const S' as 'this' argument discards qualifiers [-fpermissive]
             it->set_c();

Ok, makes sense. As some people have told me, you should use the keyword mutable as this is not captured as a const and iterator it should be modifiable now (or atleast what I'm expecting):

      void set_fn() {
        f = [this]() mutable {
            auto it = set_of_S.begin();
            it->set_c();
        };
      } 

This doesn't compile. This part doesn't make sense to me. So a member function cannot modify captured this inside lambda, but if you try to directly modify S::c inside the lambda compiler thinks that is okay. What? Doesn't make sense to me.

When I change:

void set_c() { c = 222; }

to

void set_c() const { c = 222; }

It will finally compile, but I don't like the solution, because we had to modify the original function signature just because the lambda won't accept it and it makes it less readable. I see lambdas as a tool and not something you have to design against. I have tried placing mutable keyword all over the place, but can't get it to compile. And I think there should be a way to permit member function to modify it's own state inside lambda.

Am I missing something or is this a compiler bug?

Here is the problematic code in wandbox: https://wandbox.org/permlink/qzFMW6WIRiKyY3Dj

I know this has been asked in: error: passing xxx as 'this' argument of xxx discards qualifiers but answers won't discuss on using mutable which to my understanding should solve these kind of situations.

OVOT
  • 33
  • 3
  • you don't modify the pointer `this`, so `mutable` the lambda is useless. – Jarod42 Sep 15 '21 at 15:32
  • 2
    Not a bug. `std::set::begin()` returns a [constant iterator](https://en.cppreference.com/w/cpp/container/set/begin#Notes). – Drew Dormann Sep 15 '21 at 15:37
  • @drew-dormann Why does ```it->c = 300``` then work if you uncomment ```it->set_c()```? https://wandbox.org/permlink/O9vFnhEO1RKZ9YjX – OVOT Sep 15 '21 at 16:12
  • 1
    @OVOT `it->c` works because `c` is mutable. But `it->set_c()` doesn't because `set_c` is a non-const function, so you can't call it on a const object – Kevin Sep 15 '21 at 16:13
  • @Kevin I was assuming that there is a way of assisting the compiler in a way that "this function modifies const object and it is ok" but I guess not then. – OVOT Sep 15 '21 at 16:26
  • 1
    The way you tell the compiler that is by marking the function as `const`, to say "this function can be called on a const object" – Kevin Sep 15 '21 at 16:28

1 Answers1

4

Elements of a std::set<T> are unmodifiable - set_of_S.begin() returns a constant iterator: cppreference

Because both iterator and const_iterator are constant iterators (and may in fact be the same type), it is not possible to mutate the elements of the container through an iterator returned by any of these member functions [begin/cbegin].

That means that the element pointed to by the iterator it is const, so you can't call a non-const function such as set_c on it. it->c = 300 still works because you've made c mutable. It has nothing to do with the lambda you're calling this in being mutable or not.

Kevin
  • 6,993
  • 1
  • 15
  • 24
  • Yes, maybe the standards says so. But isn't it ironic that making ```set_c``` ```const``` function actually allows you to modify the value pointed by an iterator. I think it's very unintuitive. You are true that lambda had nothing to do here, though. – OVOT Sep 15 '21 at 16:52
  • You're can shoot yourself in the foot in many ways in C++. You told the compiler that `c` was mutable, so it doesn't have a problem letting you change it inside a const function. In this case it's OK because `c` isn't part of the comparison for the set, so you aren't breaking anything. – Kevin Sep 15 '21 at 16:56