4

edit: this question should have not been closed, if you look at the answers you will see they are totally different(old question has no mention of C++17).

I was reading a PVS blog post where they mention the following bug.

(reduced)

std::map<int,int> m;
m[7]=5;
auto val = 15;
if (!m.contains(val)){
    m[val] = m.size(); // bug here
}

According to blog post this is buggy. I always thought that operator [] call for map is a function call so .size() is sequenced before [] because functions act as sequence point.

So why is this a bug?

note: I know sequence points do not exist since C++11, but I use them since new wording is much harder for me to understand.

Community
  • 1
  • 1
NoSenseEtAl
  • 28,205
  • 28
  • 128
  • 277
  • In the link provided by @cigien [this particular answer](https://stackoverflow.com/a/18040079/2296458) splits the hair regarding sequence points. – Cory Kramer May 01 '20 at 16:24
  • 3
    This strikes me as a mistake in the PVS blog post. The sequencing might be unspecified, but that's different from UB. – s3cur3 May 01 '20 at 16:28
  • 1
    _"I always thought that operator [] call for map is a function call so .size() is sequenced before [] because functions act as sequence point"_ Assignment of ints doesn't though – Asteroids With Wings May 01 '20 at 16:31
  • regarding comments: this is what blog says : You just don't know if the size function will be called before or after adding the new element. @cigien I have read the linked answer and it does not explain my problem. – NoSenseEtAl May 01 '20 at 16:41
  • @AsteroidsWithWings I think you answer my question... feel free to make it an answer... – NoSenseEtAl May 01 '20 at 16:47
  • 3
    The behavior of this has changed recently. Before C++17 this was unspecified behavior. In C++17 left hand side is guaranteed to be sequenced before the right hand side making it defined bebhavior. I left a comment on the possible dupe target asking the accepted answer to update so it would be complete for you. – NathanOliver May 01 '20 at 16:56
  • 1
    @s3cur3 if `m[val]` and `m.size()` would be indeterminately sequenced then you would be right. But they are unsequenced and it's UB – bolov May 01 '20 at 17:04
  • @AsteroidsWithWings Sorry, I had the behavior reversed. The second and third sentences [here](https://timsong-cpp.github.io/cppwp/expr.ass#1) say right happens before left, so 1 is the expected behavior. – NathanOliver May 01 '20 at 17:18
  • I am happy to see so many close votes on a question where I actually learned something new(IDK that = is a sequence point in C++17)... \s I may be biased since it is my question, but I feel SO is too trigger happy wrt close. – NoSenseEtAl May 01 '20 at 17:47

1 Answers1

5

Pre C++17

§ 1.9 Program execution [intro.execution] (n3690 c++14 draft)

  1. Except where noted, evaluations of operands of individual operators and of subexpressions of individual expressions are unsequenced.

and 5.17 [expr.ass] doesn't mention any sequencing between the operands of built-in assignment. So the evaluation of the two operands of the built-in assignment operator = is unsequenced with regards to each other.

m[val] and m.size() can be evaluated in any order (can even overlap - interleaved the CPU instructions).

Considering:

  • m[val] has a side effect of modifying the map's size (a scalar)

  • the value computation of m.size() accesses the map's size

§ 1.9 Program execution [intro.execution] (n3690 c++14 draft)

  1. [...] If a side effect on a scalar object is unsequenced relative to either [...] or a value computation using the value of the same scalar object, the behavior is undefined.

So yes, the behavior is indeed Undefined.

C++17

§8.5.18 Assignment and compound assignment operators [expr.ass] (n4713 C++17 draft)

  1. The assignment operator (=) [...] The right operand is sequenced before the left operand.

So the behavior is defined. m.size() will be evaluated before m[val]

bolov
  • 72,283
  • 15
  • 145
  • 224