2

I am implementing analogue of std::map called expiring_map which is based on boost::multi_index::multi_index_container. Idea is simple: when there is new insert to the expiring_map there will be check up for expired elements and removal if they are present.

I want to have some similar interface to the std::map:

template <class Clock, class Key, class T, class Compare = std::less<Key>>
class expiring_map
{
    explicit expiring_map(clock_type cl, duration expiry_duration);

    void insert_or_assign(value_type v);

    iterator begin();
    iterator end();
    const_iterator begin() const;
    const_iterator end() const;

    // size(), clear() and lookup-methods are omitted for simplicity
};

I also want for next code to be valid

    auto clock = std::make_shared<std::chrono::steady_clock>();
    expiring_map<decltype(clock), int, std::string> m{ clock, std::chrono::seconds{ 1 } };
    m.insert_or_assign({ 1, "value" });
    const auto b = m.begin();
    auto v = std::move(b->second);
    EXPECT_EQ(v, "value");
    EXPECT_TRUE(b->second.empty());

Since all elements for boost::multi_index::multi_index_container indices are considered non-mutable the only way (according to this answer) for me to reach desired results is to specify as value_type of expiring_map next struct

struct value_type {
    const Key first;
    mutable T second;
};

Because using value_type = std::pair<const Key, mutable T> is not valid c++ expression as keyword mutable is about storage duration and not about the type. From https://en.cppreference.com/w/cpp/language/cv

The C++ language grammar treats mutable as a storage-class-specifier, rather than a type qualifier, but it does not affect storage class or linkage.

Question

My problem with this solution is my const overloads for begin() and end() are not really const and next code is compiling:

using subject_type = ::expiring_map<std::shared_ptr<std::chrono::steady_clock>, int, std::string>;

void foo(const subject_type& s) {
   s.begin()->second = "new";
}

How can I change my implementation for const overloads of begin() and end() to achieve compilation error but preserve these methods (to still be able to iterate with range-based-for using const expiring_map&)?

Link to godbolt with current implementation and tests

What have I tried

I have tried using different extractor like

    struct extract_value_type_as_const_ref {
        [[nodiscard]] std::pair<const Key&, const T&> operator()(const node_type& n) const { return { n.second.first, n.second.second }; }
    };

    using const_iterator = decltype(boost::make_transform_iterator<extract_value_type_as_const_ref>(std::declval<underlying_type>().template get<by_value_type>().cbegin()));

but ::testing::ElementsAre requires for result of *(map.begin()) to be convertible to value_type but I really don't want to copy all of constructors of std::pair for my value_type

SherAndrei
  • 528
  • 3
  • 14
  • For those following along at home on a Macintosh (rather than the link to Matt Godbolt's online compiler), you may need `brew install googletest` and `clang++ -std=c++17 -lgtest -lgmock a.cpp`. – Eljay Jun 17 '23 at 16:08
  • Can you do this: `const_iterator begin() const = delete;`? – Eljay Jun 17 '23 at 16:19
  • @Eljay, unfortunately, no. `::testing::ElementsAre` requires const overloads. I definitely can live without `::testing::ElementsAre`, but if someone tomorrow will have to iterate through my container using range-based-for, there will be unexpected compilation error – SherAndrei Jun 17 '23 at 16:43
  • *How can I change my implementation for const overloads of begin() and end() to achieve compilation error?* I thought you **wanted** to achieve compilation error. – Eljay Jun 17 '23 at 17:06
  • @Eljay, yep, you are right, I've updated the question to clarify my desires :) – SherAndrei Jun 17 '23 at 18:15
  • 1
    `auto v = std::move(b->second);` does not make sense. `b` is a `std::string`, not a pointer. – Eljay Jun 17 '23 at 18:49
  • @Eljay, no, it is not, as it is compiles on Compiler Explorer. You can let your compiler check for you with `static_assert(std::is_same_v, typename subject_type::iterator>);` or for `b->second`: `static_assert(std::is_same_vsecond), std::string>);` or without `static_assert`s by specifying type by yourself: `std::string v = std::move(b->second);` – SherAndrei Jun 17 '23 at 20:40
  • The `auto v = std::move(b->second);` does not compile for me. I don't think the code that you *want to be valid* will be valid. – Eljay Jun 17 '23 at 23:27
  • @Eljay, oh, now I get it. There was an error in code snippet in the question as you noticed indeed, my apologies. I've misunderstood your remark since in attached [link](https://godbolt.org/z/xW1rx4or7) correct code was shown. I've edited the question, thank you for your patience – SherAndrei Jun 18 '23 at 01:26
  • 1
    I would not try to make this implicitly/silently work. To `value_type`, I'd add a mutating accessor to second, and impose upon the callsite to explicitly access the second value as mutating. `T& mut_second() const { auto& self = const_cast(*this); return self.second; }`. That puts all the dynamite in the caller's hands. (Up to you if `second` should be declared `mutable`, but I don't think so.) There be dragons here. – Eljay Jun 18 '23 at 13:02

1 Answers1

0

The second member in your value_type should not be mutable. This is the fix:

struct value_type {
    const Key first;
    T second;
};

Making this change makes your assertion on Compiler Explorer pass.

Maybe you've misunderstood something about the value_type of maps in general. A std::map<Key, Value> will store a value_type = std::pair<const Key, Value>.

  • The Key has to be const. Otherwise, we would be able to mutate the key without also changing the position of the pair in the map, which breaks the data structure.
  • The Value is not mutable. A const std::map will expose const value_type&, and the Value stored inside that pair is then also const.1)

Furthermore, it makes sense that std::pair<const Key, mutable T> wouldn't compile. mutable is a property of data members (like private or static), it's not part of the a type.

In general, mutable is only meant to be used for data members which have to be mutable under all circumstances, even inside of const objects. For example, a std::mutex member is typically mutable, because a const std::mutex is unusable, and you need to be able to use it regardless.


1) It is possible that the Value stored inside is not a const object, but a mutable object. Technically, you could still mutate it through const_cast then. However, this cannot be relied upon.

Jan Schultke
  • 17,446
  • 6
  • 47
  • 96
  • Sorry for the inconvenience, I've updated the link in the question, added desired test, in which removing `mutable` does not save the situation – SherAndrei Jun 17 '23 at 18:12
  • > `mutable` is only meant to be used for data members which have to be mutable under all circumstances, Yeah, i'm aware, but I want to find a way to be able to modify elements in `boost::multi_index::multi_index_container` using only my iterator without assisting methods like `modify`, `replace` which are offered by `boost` – SherAndrei Jun 17 '23 at 18:18