code review 6909060: spec: calling delete on a nil map is a no-op (issue 6909060)

[g...@golang.org]

Reviewers: rsc, r, iant, ken2,

Message:
Hello rsc, r, iant, ken2 (cc: golan...@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go


Description:
spec: calling delete on a nil map is a no-op

This is language change. It is a backward-compatible
change but for code that relies on a run-time panic
when calling delete on a nil map (unlikely).

Fixes issue 4253.

Please review this at https://codereview.appspot.com/6909060/

Affected files:
M doc/go_spec.html


Index: doc/go_spec.html
===================================================================
--- a/doc/go_spec.html
+++ b/doc/go_spec.html
@@ -1,6 +1,6 @@
<!--{
"Title": "The Go Programming Language Specification",
- "Subtitle": "Version of December 6, 2012",
+ "Subtitle": "Version of December 10, 2012",
"Path": "/ref/spec"
}-->

@@ -5096,9 +5096,8 @@
</pre>

<p>
-If the element <code>m[k]</code> does not exist, <code>delete</code> is
-a no-op. Calling <code>delete</code> with a nil map causes a
-<a href="#Run_time_panics">run-time panic</a>.
+If the map <code>m</code> is <code>nil</code> or the element
<code>m[k]</code>
+does not exist, <code>delete</code> is a no-op.
</p>

[Russ Cox]

LGTM but wait for others

Background: the spec used to be self-conflicting. It said this part
about panic but also said:
A nil map is equivalent to an empty map except that no elements
may be added.

This edit brings the delete semantics in line with that general rule.

Russ

[Brad Fitzpatrick]

I would prefer deletes on a nil map explode on me... I would assume that's always my bug.

Seems weird to permit an attempt to mutate a immutable thing.

Instead of:

"A nil map is equivalent to an empty map except that no elements
may be added."

I would prefer:

"A nil map is equivalent to an empty map except that it may not be {mutated,modified,written}."

Similar to how we permit reading a nil channel, but not sending on it or closing it.

[Robert Griesemer]

The current behavior seems less consistent to me then the proposed one
(delete on nil maps in no-op); even w/ your proposed phrasing change (

"A nil map is equivalent to an empty map except that it may not be

{mutated,modified,written}." ).

For one, nil maps behave exactly like empty maps now (deletion,
lookup, iteration). It seems odd to make an exception for delete. We
have been there before and relaxation for nil maps (no run-time error)
really simplified code and removed unnecessary if's.

Also, if you require a runtime panic for delete on nil maps, one might
as well require a run-time panic for deletion of a non-existing
element in a non-nil map - this seems to be the same kind of "bug".

The comparison with close seems not correct. There is a signal
transmitted when closing a channel, as with sending on a channel.
Neither receiving from a channel not deletion from a map requires a
channel or map to be present to decide how to proceed.

- gri

[Russ Cox]

On Mon, Dec 10, 2012 at 1:59 PM, Brad Fitzpatrick <brad...@golang.org> wrote:
> I would prefer deletes on a nil map explode on me... I would assume that's
> always my bug.

My experience is the opposite. Most of the time I do delete(x, key) I
have to put in a special case for x being nil.

[roger peppe]

i agree. i've wanted this change before, and i'm definitely +1 on it now.

[Robert Griesemer]

PTAL
Any others?
- gri

[ia...@golang.org]

LGTM

(Actually I don't really care.)

https://codereview.appspot.com/6909060/

[g...@golang.org]

*** Submitted as
https://code.google.com/p/go/source/detail?r=d09a8b21b517 ***

spec: calling delete on a nil map is a no-op

This is language change. It is a backward-compatible
change but for code that relies on a run-time panic
when calling delete on a nil map (unlikely).

Fixes issue 4253.

R=rsc, r, iant, ken, bradfitz, rogpeppe
CC=golang-dev
https://codereview.appspot.com/6909060


https://codereview.appspot.com/6909060/