On Tue, Feb 23, 2010 at 12:29 AM, Brice Figureau <
[email protected]> wrote:

> On Mon, 2010-02-22 at 15:59 -0800, Markus Roberts wrote:
> > Brice (and anyone else who wants to kibitz/opine) --
>
> Since you sent it to me only, I'm not sure anyone beside me will
> answer :-)
> I'm CC:ing puppet-dev so that other people can chime in.
>

Thanks.  That had been my intent.

>     $h += {a => z}
>
> This one should clearly be forbidden. It shouldn't be possible to
> rewrite an hash value of an existing key.
>

Ok, that's clear and consistent.


> > [Hash value augmentations escaping scope]
>


> This one looks problematic too. All modifications to h inside c1 should
> have been done on a copy in the c1 scope not on the original hash.
> I bet there is something wrong in my patch.
> It looks like the hash instance is shared in all the scopes instead of
> being duplicated (why does it work for arrays then?).
>

Again, clear and consistent (and something that I expect can be cleaned up
easily in the reconciliation).  I am wondering, though, if this
functionality isn't part of the appeal of hashes (e.g. in the fragment
collection use case).  If so, one possible resolution (and the only one I
can think of that isn't worse than the problem) would be to have the whole
hash, with all the augmentations, available everywhere that the hash was
available--in other words, if we want "collection" of hash values we can
make it more semantically palatable by at least removing the sequentiality.

>       * In the futures-branch I consider this array behaviour
>         incorrect in that it is order dependent within c1, and aim
>         for:
> $a = [a,b]
>
> class c1 {
>     notice "2: a = $a"
>     $a += [c,d]
>     notice "3: a = $a"
> }
>
> notice "1: a = $a"
> include c1
> notice "4: a = $a"
> --------------------------------------------------------
> notice: Scope(Class[main]): 1: a = ab
> notice: Scope(Class[c1]): 2: a = abcd
> notice: Scope(Class[c1]): 3: a = abcd
> notice: Scope(Class[main]): 4: a = ab

I don't see what the issue is.
> To me it's exactly the same as writing:
>
> $a = "test"
> class c1 {
>  $a = "toto ${a} titi"
> }
> include c1
>
> That is we're shadowing the main $a in the scope of c1.
>

Exactly.  I wasn't saying that the ab/abcd/abcd/ab behaviour was incorrect,
I was saying that it is what I'm aiming for in the futures branch.  The
ab/ab/abcdef/ab behaviour is what I was objecting to.

>       * If multiple extensions to hashes are allowed, what's the
> >         rationale for prohibiting them for arrays (on the assumption
> >         that all references that see any of the changes see all of
> >         them)?  One possible answer is that arrays, being ordered,
> >         would then expose execution order.
>
> When I originally implemented the append operator, I wanted in fact to
> simplify:
>
> $a = "string"
> class newscope {
>  $a = "new string using ${string}"
> }
>
> Since it is possible to create a new variable only once, I used the
> exact same semantic for the append operator (both for arrays and regular
> variables).
>
> Now, since it seems to be possible to extend hashes more than one time
> in a definite scope, I did it wrong for them.
>
> I'm somewhat completely undecided about what behaviour for hash is
> correct or not, about mulitple appending. The conservative guy deep
> inside me would tend to say that it should also be fordidden.
>

I can see that (I'm similarly conflicted); but what I can't see in either
case is _why_.   We can, for example, add multiple variables to a scope, and
even add multiple keys/value pairs to a hash with +=, so why is h[k]=v
special?

I'm suspecting that the real problem is that it's a kludge stuck in to
prevent:

    a += [1]
    use a
    a += [2]
    use a

from exposing the order of evaluation dependence that's present without
futures.


> >       * Should hashes "leak" changes to the outer scope?  On the one
> >         hand, this is AFAIK very different from how anything else in
> >         puppet works; on the other it is the basis for some of the
> >         most compelling use cases for hashes.
>
> I'd say they shouldn't. Variables don't do that, array don't do that,
> hash sould not do that.
>
> > There's more, but this should get us started.
>
> Indeed :-)
>
> Now, I have a question: let's say I'm fixing the holes you found in the
> hash patch, what is the best way to submit the fixes?
> Among the possibilities are:
>  * producing a new testing based branch, still rebased on the same
> commit as now
>  * producing a master branch
>  * something else?
> (choose your poison).
>

I'll have to think about that one.  For right now I'm still wrestling with
"what should it do" and not worrying so much over the "how to get the
changes in" part (at least, wit this part of my brain).


> Thanks for your analysis of the issues,
>

Ditto. :)

-- Markus

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to