fixes #40: account for mutation of map in getOrInsertComputed callbacks

[michaelficarra]

Fixes #40. Just checks for presence of the key again after calling the callback. @dead-claudia suggested we could have a note about a potential optimisation here, but I'll leave that to the editor group to decide upon integration.

Closes #73.

[bakkot]

I would strongly prefer that this throw rather than returning the value which was inserted while the callback was running. If that's not acceptable, it should return the value produced by the callback. In no circumstances should it call the callback and then silently not insert that value.

[michaelficarra]

@bakkot Read it again, it sets the value and returns that value, regardless of what it was set to in the interim.

[bakkot]

Ah, good good. Well, I'd still prefer an error.

Incidentally, Java says:

Non-concurrent implementations should override this method and, on a best-effort basis, throw a ConcurrentModificationException if it is detected that the mapping function modifies this map during computation.

[michaelficarra]

I'm also fine with locking and throwing. This design question should probably be posed to committee @dminor.

[bakkot]

Locking requires a lot more implementation work. Maybe it's easy in practice and doesn't slow down other uses, in which case that would be ideal, but I was envisioning just replacing the "if the value is already present after calling the callback, then update its entry" parts of this PR with "[...], then throw".

Actually, on further thought, I am against locking even if it is easy to do because right now map.set(foo, bar) never throws, and I don't want people to have to think about the fact that it might. The place the error should originate is at the getOrInsertComputed call.

[michaelficarra]

I'll just open a second PR right now to make it easier for people to compare.

edit: See #73.

[dminor]

I've requested a larger timebox, and I'll add this issue to the slides for discussion in committee.