Assignment in C++ occurs despite exception on the right side

Question

I have some (C++14) code that looks like this:

map<int, set<string>> junk;
for (int id : GenerateIds()) {
    try {
        set<string> stuff = GetStuff();
        junk[id] = stuff;
    } catch (const StuffException& e) {
        ...
    }
}

This works. Sometimes GetStuff() throws an exception, which works fine, because if it does, I don't want a value in the junk map then.

But at first I'd written this in the loop, which doesn't work:

junk[id] = GetStuff();

More precisely, even when GetStuff() throws an exception, junk[id] is created (and assigned an empty set).

This isn't what I'd expect: I'd expect them to function the same way.

Is there a principle of C++ that I've misunderstood here?

Also see [c++ map exception safe insert](https://www.google.com/search?q=c%2B%2B+map+exception+safe+insert) and [Making operations on standard-library containers strongly exception safe](https://pdfs.semanticscholar.org/2a6a/30b9f7cd82ef29b64fbecae10463d533a6da.pdf), [Exception Safety: Concepts and Techniques](http://www.stroustrup.com/except.pdf), etc. — jww, Nov 05 '18 at 12:59
See [Order of evaluation of assignment statement in C++](https://stackoverflow.com/q/33598938/1708801) also related [Does this code from “The C++ Programming Language” 4th edition section 36.3.6 have well-defined behavior?](https://stackoverflow.com/q/27158812/1708801) and [What are the evaluation order guarantees introduced by C++17?](https://stackoverflow.com/q/38501587/1708801) — Shafik Yaghmour, Nov 05 '18 at 14:07
Just to clarify for future readers with similar problems: **no assignment happens here**. `junk[id]` creates a new `set`, yes, but that is using the default constructor. That's why the set is empty. This empty set would have been used as the object to assign to, if `GetStuff()` would have succeeded. But the exception thrown is precisely why no assignment happens. The set is left in its default state. It is a proper C++ object, and you can call its members normally. I.e. `junk[id].size()` will be 0 afterwards. — MSalters, Nov 05 '18 at 15:46
@MSalters Good clarification! I used "assignment" too loosely (but "default constructed" was maybe a bit heavy for the question title ;-). — jma, Nov 05 '18 at 16:03
To make this even more safe (and more efficient), the assignment should be `junk[id] = std::move(stuff);` . — David Hammen, Nov 06 '18 at 10:01

score 96 · Accepted Answer · edited Nov 06 '18 at 18:18

96

Before C++17 there was no sequencing between the left- and right-hand side of assignment operators.

It's first in C++17 that explicit sequencing was introduced (right-hand side is evaluated first).

That means the evaluation order is unspecified, which means it's up to the implementation to perform the evaluation in the order in which it wants, and in this case it evaluates the left-hand side first.

See this evaluation order reference for more details (especially point 20).

edited Nov 06 '18 at 18:18

cat

3,442
3
28
55

answered Nov 05 '18 at 12:58

Some programmer dude

363,249
31
351
550

4

If I understand the reference correctly the first sentence in the answer could be shortened by removing "overloaded", as right-hand-side is evaluated first for both overloaded and non-overloaded assignment operators in C++17. – Hans Olsson Nov 05 '18 at 14:01

score 17 · Answer 2 · edited Nov 06 '18 at 09:09

std::map::operator[]

Returns a reference to the value that is mapped to a key equivalent to key, performing an insertion if such key does not already exist.

junk[id] causes the above mentioned insertion and after that has already happened GetStuff() throws. Note that in C++14 the order in which these things happen is implementation defined so with a different compiler your junk[id] = GetStuff(); may not do the insertion if GetStuff() throws.

score 12 · Answer 3 · answered Nov 05 '18 at 12:58

12

You're misunderstanding how operator[] works on std::map.

It returns a reference to the mapped item. Therefore, your code is first inserting a default item in that position and then invoking operator= to set a new value.

To make this work the way you expect, you'll need to use std::map::insert (*):

junk.insert(std::make_pair(id, GetStuff()));

Caveat: insert will only add the value if id is not already mapped.

answered Nov 05 '18 at 12:58

paddy

52,396
6
51
93

6

i dont think there is any misconception about how `operator[]` works involved here. Also from your answer its not clear why `operator=` evaluates the left hand side, even if the right hand side throws an exception – 463035818_is_not_a_number Nov 05 '18 at 13:03
If there is any misunderstanding, it would for me be a misunderstanding for a[b]=...; not just for a[...]. – Hans Olsson Nov 05 '18 at 15:04
8

@user463035818: The questioner expresses a belief that an assignment occurred, which strongly suggests they are unaware that evaluating the `operator[]` would create an empty set for `junk[id]` on its own. – user2357112 supports Monica Nov 05 '18 at 18:27
1

Upvoted for suggesting a way to write it without first default-constructing an entry, regardless of whether there's a "misunderstanding" or not. – Peter Cordes Nov 06 '18 at 01:41

Assignment in C++ occurs despite exception on the right side

3 Answers3

Linked