Is this as dangerous as I think it is?

roger peppe roger.peppe at canonical.com
Thu Oct 22 13:26:28 UTC 2015


By the language-defined semantics of append, this is fine.
It would actually be quite hard to define isAliveDoc such
that it had cap != len.

That said, it would be easy to write:

    // addAsserts returns the result of appending elems to docs.
    // It does not mutate docs.
    func addAsserts(docs bson.D, elems ...bson.DocElem) bson.D {
        var r bson.D
        r = append(r, base...)
        return append(r, elems...)
    }

or similar, which is semantically equivalent and may well compile to
the same code in the end.


On 22 October 2015 at 13:42, John A Meinel <john.meinel at canonical.com> wrote:
> While doing a review, I came across this snippet of code:
> asserts = append(isAliveDoc, asserts...)
>
> It took me a while to understand why the thing that is being appended is at
> the beginning. I realize it probably wants to insert it at 0, but I thought
> it was an object, not a slice, so how could it go at the beginning. But
> digging shows it is actually a slice:
>
> https://github.com/juju/juju/blob/master/state/life.go#L36
> var isAliveDoc = bson.D{{"life", Alive}}
>
> and bson.D:
> http://bazaar.launchpad.net/+branch/mgo/v2/view/head:/bson/bson.go#L118
> type D []DocElem
>
> Doesn't that mean that every time we call something like this, we are
> potentially mutating the isAliveDoc statement?
>
> I realize it probably "works" because isAliveDoc has capacity 1 and thus
> must be reallocated everytime you append, and thus we don't end up with an
> isAliveDoc that actually carries arround all the assertions from everything
> else.
>
> but that seems very "assume no bad side effects" rather than valid code.
>
> Thoughts?
>
> John
> =->
>
>
> --
> Juju-dev mailing list
> Juju-dev at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/juju-dev
>



More information about the Juju-dev mailing list