FAQ
Reviewers: rsc, r, iant, ken2,

Message:
Hello rsc, r, iant, ken2 (cc: golang-dev@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>

Search Discussions

  • Russ Cox at Dec 10, 2012 at 6:42 pm
    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 at Dec 10, 2012 at 6:59 pm
    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.

    On Mon, Dec 10, 2012 at 1:42 PM, Russ Cox wrote:

    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
  • Russ Cox at Dec 10, 2012 at 7:36 pm

    On Mon, Dec 10, 2012 at 1:59 PM, Brad Fitzpatrick 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 at Dec 11, 2012 at 12:12 pm

    On 10 December 2012 19:36, Russ Cox wrote:
    On Mon, Dec 10, 2012 at 1:59 PM, Brad Fitzpatrick 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.
    i agree. i've wanted this change before, and i'm definitely +1 on it now.
  • Robert Griesemer at Dec 12, 2012 at 7:13 pm
    PTAL
    Any others?
    - gri
    On Mon, Dec 10, 2012 at 10:42 AM, Russ Cox wrote:
    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
  • Gri at Dec 12, 2012 at 9:08 pm
    *** 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/
  • Iant at Dec 12, 2012 at 9:10 pm
    LGTM

    (Actually I don't really care.)

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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 10, '12 at 6:23p
activeDec 12, '12 at 9:10p
posts8
users5
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase