Non-const dereferencing

[ozars]

Hi, thanks for this great library.

I realized iterator doesn't have non-const variant of operator*(), which is also explicitly stated in the README. value() function is recommended instead, but it doesn't play well with range-loops where values need to be mutated.

Example code:

#include "tsl/ordered_map.h"

int main() {
    tsl::ordered_map<int, int> m;
    for (auto& v: m) {
        int& value = v.second;
    }
}

$ g++ --version 
g++ (Debian 8.3.0-6) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ g++ -std=c++11 test.cpp -o test
test.cpp: In function ‘int main()’:
test.cpp:6:24: error: binding reference of type ‘int&’ to ‘const int’ discards qualifiers
         int& value = v.second;
                      ~~^~~~~~

Is there a specific reason for disallowing it or is it just that it's not implemented? Thanks.

[JTurnerJ]

You're awesome. Thank you for the reply I will implement your solution.

Thanks again

On Wed, Mar 11, 2020 at 2:27 PM Omer Ozarslan notifications@github.com wrote:

Hi, thanks for this great library.

I realized iterator doesn't have non-const variant of operator*(), which is also explicitly stated in the README. value() function is recommended instead, but it doesn't play well with range-loops where values need to be mutated.

Example code:

include "tsl/ordered_map.h"

int main() {

tsl::ordered_map<int, int> m;

for (auto& v: m) {

    int& value = v.second;

}

}

$ g++ --version g++ (Debian 8.3.0-6) 8.3.0 Copyright (C) 2018 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ g++ -std=c++11 test.cpp -o test test.cpp: In function ‘int main()’: test.cpp:6:24: error: binding reference of type ‘int&’ to ‘const int’ discards qualifiers int& value = v.second; ^~~~

Is there a specific reason for disallowing it or is it just that it's not implemented? Thanks.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Tessil/ordered-map/issues/28, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALILYTIE63RWUGGY6ZEIPSDRG762VANCNFSM4LF7AYSQ .

-- Jerry Turner jerry.turner.1@gmail.com | (818) 632-3088

[Tessil]

Hi,

Yes the current implementation doesn't play really well with range-based for loop but there is effectively a specific reason for this. See issue 23 of the hopscotch-map project which describes the reason that also applies here.

[ozars]

Thanks. Would it make sense to use a template parameter like MutableValueType in ordered_hash whose default value is std::pair<const Key&, T&> for implementing MutableValueType operator*()?

[Tessil]

Returning a pair of references could be a possibility, but I prefer to keep returning a const reference to the stored pair for now. Adding an extra template class parameter to make it configurable is a possibility, but it would bloat the interface for a minor advantage.