std::map sequencing of operator[] and size()

Question

I have a std::map and trying to fill it with pairs (name, id). The id field is simply generated from map's size(). Here's a simplified version:

#include <iostream>
#include <map>

struct A {
    std::string name;
    int id;
    A(const std::string &s) : name(s), id(-1) { }
};

class Database {
    std::map<std::string, int> ids;
public:
    void insert(A *item) {
        ids[item->name] = item->id = ids.size();
    }
    void dump() const {
        for (std::map<std::string, int>::const_iterator i = ids.begin(); i != ids.end(); i++)
            std::cout << i->second << ". " << i->first << std::endl;
    }
};

int main(int argc, char **agrv) {
    A a("Test");
    Database db;
    db.insert(&a);
    db.dump();
    return 0;
}

The problem is that different compilers treat the ids[item->name] = item->id = ids.size() part differently. Clang++ produces

item->id = ids.size(); // First item gets 0
ids[item->name] = item->id;

when g++ does something like

ids.insert(std::pair<std::string, int>(item->name, 0));
item->id = ids.size(); // First item gets 1
ids[item->name] = item->id;

So, is this code valid (from the STL perspective) or it is as evil as i = ++i + ++i?

Change your insert function to take only a string, insert ids.size() directly into map, and return a reference to the item just inserted. — Neil Kirk, Aug 01 '13 at 15:20
If the id is simply an index, what is the point in placing the items into std::map ? Why not simply store them into an std::vector or array[] and access them from the index later? — Shootfast, Aug 01 '13 at 15:15

score 9 · Accepted Answer · edited May 23 '17 at 10:31

9

ids[item->name] = item->id = ids.size();

Without a sequence point separating the two calls, the compiler is free to evaluate operator[] and size() in any order it likes. There is no guarantee that size() will be called before operator[].

edited May 23 '17 at 10:31

Community

1
1

answered Aug 01 '13 at 15:11

John Kugelman

349,597
67
533
578

2

And to answer his last question: the code is valid, but parts of the behavior is unspecified. (And it's not as evil as `i = ++i + ++i`.) – James Kanze Aug 01 '13 at 15:20
2

Sequence points are so 2003. Now we have *sequenced-before* ordering relationships... this example is *indeterminately sequenced*, but `i = ++i + ++i` is *unsequenced* and therefore more evil – Ben Voigt Aug 01 '13 at 15:42

score -1 · Answer 2 · answered Aug 01 '13 at 16:23

-1

change ids[item->name] = item->id = ids.size(); to

item->id = ids.size();
ids[item->name] = item->id;

answered Aug 01 '13 at 16:23

BlackMamba

10,054
7
44
67

std::map sequencing of operator[] and size()

2 Answers2