[PATCH] First try of null typeset

[PATCH] First try of null typeset

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

This achieves most of what Bart Schaefer's version achieves, except no extra hacks are needed, and
integer and floats are not changed.

This most definetly is not close to the final solution, but shows what can be achieved by narrowing
down the functional changes to two functions.

In my opinion a separate concept of "null" variable will be needed, and should be separate from
PM_UNSET, since that changes a lot of behavior.

Also, I don't think $empty[(i)] should return nothing, so probably paramsubst() would need to be
tuned, not simply set vunset=1.

Addtionally, this patch doesn't change the behavior of the private module.

 Src/params.c | 23 ++++++++++++++++++++++-
 Src/subst.c  |  2 +-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/Src/params.c b/Src/params.c
index 122f5da7d..7dbe67bd1 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -3697,6 +3697,27 @@ unsetparam_pm(Param pm, int altflag, int exp)
     return 0;
 }
 
+/**/
+mod_export int
+isnull(Param pm)
+{
+    switch (PM_TYPE(pm->node.flags)) {
+    case PM_SCALAR:
+	if (pm->gsu.s->getfn == strgetfn && !pm->u.str)
+	    return 1;
+	break;
+    case PM_ARRAY:
+	if (pm->gsu.a->getfn == arrgetfn && !pm->u.arr)
+	    return 1;
+	break;
+    case PM_HASHED:
+	if (pm->gsu.h->getfn == hashgetfn && !pm->u.hash)
+	    return 1;
+	break;
+    }
+    return 0;
+}
+
 /* Standard function to unset a parameter.  This is mostly delegated to *
  * the specific set function.
  *
@@ -5955,7 +5976,7 @@ printparamnode(HashNode hn, int printflags)
 	}
     }
 
-    if ((printflags & PRINT_NAMEONLY) ||
+    if ((printflags & PRINT_NAMEONLY) || isnull(p) ||
 	((p->node.flags & PM_HIDEVAL) && !(printflags & PRINT_INCLUDEVALUE)))
 	quotedzputs(p->node.nam, stdout);
     else {
diff --git a/Src/subst.c b/Src/subst.c
index 8f5bd355e..42d18d58d 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -2532,7 +2532,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags,
 			     (wantt ? -1 :
 			      ((unset(KSHARRAYS) || inbrace) ? 1 : -1)),
 			     scanflags)) ||
-	    (v->pm && (v->pm->node.flags & PM_UNSET)) ||
+	    (v->pm && (v->pm->node.flags & PM_UNSET || isnull(v->pm))) ||
 	    (v->flags & VALFLAG_EMPTY))
 	    vunset = 1;
 
-- 
2.29.2

Re: [PATCH] First try of null typeset

[Bart Schaefer]

As you might be able to tell, the end of the USA long holiday weekend
has severely reduced my ability to pay attention to this.

On Tue, Dec 1, 2020 at 1:13 AM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> This achieves most of what Bart Schaefer's version achieves, except no extra hacks are needed, and
> integer and floats are not changed.

Applying the change to integers and floats is one of the reasons I
went in the direction I did.  What constitutes an "extra hack"?

> In my opinion a separate concept of "null" variable will be needed, and should be separate from
> PM_UNSET, since that changes a lot of behavior.

I don't think it's possible to introduce a separate concept of "null"
inside struct param without touching at least all the same places my
branch already did, and probably others.  I would not be unhappy to be
proven incorrect, but PM_UNSET already covers all the corner cases of
what to do when a parameter should be treated as having no value,
because that's how "local" followed by "unset" has always worked.

> Also, I don't think $empty[(i)] should return nothing, so probably paramsubst() would need to be tuned

Agree.  This may be easier than it seems, because (i)/(I) already work
on scalars: { thing="abcde"; print $thing[(I)] } yields 6.  I just ran
out of time to dig further.

> Addtionally, this patch doesn't change the behavior of the private module.

If by this you mean my branch changed something about "private" in a
way that doesn't correspond to the rest of the changes, then I've
overlooked something.

Re: [PATCH] First try of null typeset

[Felipe Contreras]

On Wed, Dec 2, 2020 at 12:48 PM Bart Schaefer <schaefer@brasslantern.com> wrote:

> On Tue, Dec 1, 2020 at 1:13 AM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > This achieves most of what Bart Schaefer's version achieves, except no extra hacks are needed, and
> > integer and floats are not changed.
>
> Applying the change to integers and floats is one of the reasons I
> went in the direction I did.

Yes, I suspect that's the way it will eventually have to be done, but
others argued "typeset -i var" should initialize it to 0. So for those
my patch already does what is needed.

> What constitutes an "extra hack"?

The one I did to fix the PM_TIED case. I sent a patch for that.

> > In my opinion a separate concept of "null" variable will be needed, and should be separate from
> > PM_UNSET, since that changes a lot of behavior.
>
> I don't think it's possible to introduce a separate concept of "null"
> inside struct param without touching at least all the same places my
> branch already did, and probably others.  I would not be unhappy to be
> proven incorrect, but PM_UNSET already covers all the corner cases of
> what to do when a parameter should be treated as having no value,
> because that's how "local" followed by "unset" has always worked.

But as you showed the code assumes PM_UNSET variables don't exist.

I could comb through to code looking for instances PM_UNSET checks,
but I'm sure I will find some instances where the code doesn't do what
we want in that case.

> > Also, I don't think $empty[(i)] should return nothing, so probably paramsubst() would need to be tuned
>
> Agree.  This may be easier than it seems, because (i)/(I) already work
> on scalars: { thing="abcde"; print $thing[(I)] } yields 6.  I just ran
> out of time to dig further.

Yes, me too. My patch did the lazy thing too and just doesn't return anything.

> > Addtionally, this patch doesn't change the behavior of the private module.
>
> If by this you mean my branch changed something about "private" in a
> way that doesn't correspond to the rest of the changes, then I've
> overlooked something.

No. Your patch may be doing the desired behavior. Presumably if
"typeset var" doesn't set a value for var, then "private var"
shouldn't either.

I'm just saying my patch doesn't do that (although it probably should).

Cheers.

-- 
Felipe Contreras

Re: [PATCH] First try of null typeset

[Roman Perepelitsa]

On Wed, Dec 2, 2020 at 10:59 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> On Wed, Dec 2, 2020 at 12:48 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > Applying the change to integers and floats is one of the reasons I
> > went in the direction I did.
>
> Yes, I suspect that's the way it will eventually have to be done, but
> others argued "typeset -i var" should initialize it to 0. So for those
> my patch already does what is needed.

I've two opinions here.

The first is that it's not a good idea to introduce a mode in which
`typeset var` declares an unset variable but `typeset -i var` doesn't.
The motivation for changing the behavior of `typeset var` is to be
more compatible with other shells. However, there is no shell where
`typeset [flag]... var` leaves or doesn't leave the variable unset
depending on flags.

My second opinion on this matter is that it's not a good idea to
change the behavior of `typeset -i var` in native mode with default
options. The benefits (if any) of this change would be too small to
justify braking user code.

These two points are independent.

Roman.

Re: [PATCH] First try of null typeset

[Felipe Contreras]

On Thu, Dec 3, 2020 at 2:44 AM Roman Perepelitsa
<roman.perepelitsa@gmail.com> wrote:
>
> On Wed, Dec 2, 2020 at 10:59 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > On Wed, Dec 2, 2020 at 12:48 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > >
> > > Applying the change to integers and floats is one of the reasons I
> > > went in the direction I did.
> >
> > Yes, I suspect that's the way it will eventually have to be done, but
> > others argued "typeset -i var" should initialize it to 0. So for those
> > my patch already does what is needed.
>
> I've two opinions here.
>
> The first is that it's not a good idea to introduce a mode in which
> `typeset var` declares an unset variable but `typeset -i var` doesn't.
> The motivation for changing the behavior of `typeset var` is to be
> more compatible with other shells. However, there is no shell where
> `typeset [flag]... var` leaves or doesn't leave the variable unset
> depending on flags.

That's *one* motivation, it's not the only motivation.

Another motivation is to do the right thing, which POSIX after decades
of discussion hasn't managed to agree on what is the right thing.

If zsh somehow managed to find the right thing, and the right thing is
for "typeset -i var" to set a value while "typeset var" doesn't, then
that would solve a lot of future problems.

Alas, I don't think that's the right thing.

> My second opinion on this matter is that it's not a good idea to
> change the behavior of `typeset -i var` in native mode with default
> options. The benefits (if any) of this change would be too small to
> justify braking user code.

Agreed, at least not now (maybe the next major version).

But it can be changed in some mode(s).

-- 
Felipe Contreras