zsh-workers
 help / color / mirror / Atom feed
* [PATCH] declarednull: felipec's approach
@ 2020-12-23 23:47 Felipe Contreras
  2020-12-27 18:27 ` Bart Schaefer
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Felipe Contreras @ 2020-12-23 23:47 UTC (permalink / raw)
  To: zsh-workers; +Cc: Bart Schaefer, Felipe Contreras

This patch merges Bart's approach with my approach, and should be
applied on top of the branch declarednull.

The main difference is that PM_DECLAREDNULL does not set PM_UNSET. So in
the two relevant places where PM_UNSET should be checked, PM_DELCAREDNULL
is checked too.

In my branch it's actually called PM_NULL because I think semantically
makes more sense. I just renamed it for this patch to minimize the diff.

A variable is initially declared with PM_NULL (same as PM_DECLAREDNULL),
then, once a value is a assigned, PM_NULL is removed.

That's it. Semantically it makes sense.

What doesn't make sense is to remove PM_DECLAREDNULL, because it's a
combination of PM_UNSET and PM_DECLARED. It makes sense to remove
PM_UNSET, but not to remove PM_DECLARED.

Why would this

  local var
  var=foobar

remove PM_DELCARED?

Additionally it's not clear why unset should clear the PM_DECLAREDNULL
flag. PM_UNSET is going to be immediately set anyway, and PM_DECLARED is
set only when there's no value (essentially PM_NULL), so it's kind of
returning to var='' (assuming there's a value).

I added a test that shows a discrepancy I found (${(t)var}) but there
could be many, may more. I only checked one instance of PM_UNSET.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Src/builtin.c      |  2 +-
 Src/params.c       | 11 ++++-------
 Src/subst.c        |  2 +-
 Src/zsh.h          |  3 +--
 Test/E03posix.ztst |  8 ++++++++
 5 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/Src/builtin.c b/Src/builtin.c
index 1e950f122..988233504 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2837,7 +2837,7 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
 	    unqueue_signals();
 	    return 1;
 	} else if (pm) {
-	    if ((!(pm->node.flags & PM_UNSET) || pm->node.flags & PM_DECLARED)
+	    if (!(pm->node.flags & PM_UNSET)
 		&& (locallevel == pm->level || !(on & PM_LOCAL))) {
 		if (pm->node.flags & PM_TIED) {
 		    if (PM_TYPE(pm->node.flags) != PM_SCALAR) {
diff --git a/Src/params.c b/Src/params.c
index c09a3eccf..8912a7f65 100644
--- a/Src/params.c
+++ b/Src/params.c
@@ -2093,8 +2093,7 @@ fetchvalue(Value v, char **pptr, int bracks, int flags)
 	if (sav)
 	    *s = sav;
 	*pptr = s;
-	if (!pm || ((pm->node.flags & PM_UNSET) &&
-		    !(pm->node.flags & PM_DECLARED)))
+	if (!pm || (pm->node.flags & PM_UNSET))
 	    return NULL;
 	if (v)
 	    memset(v, 0, sizeof(*v));
@@ -3625,7 +3624,6 @@ unsetparam_pm(Param pm, int altflag, int exp)
     else
 	altremove = NULL;
 
-    pm->node.flags &= ~PM_DECLARED;	/* like ksh, not like bash */
     if (!(pm->node.flags & PM_UNSET))
 	pm->gsu.s->unsetfn(pm, exp);
     if (pm->env)
@@ -5854,9 +5852,8 @@ printparamnode(HashNode hn, int printflags)
     Param peer = NULL;
 
     if (p->node.flags & PM_UNSET) {
-	if ((printflags & (PRINT_POSIX_READONLY|PRINT_POSIX_EXPORT) &&
-	     p->node.flags & (PM_READONLY|PM_EXPORTED)) ||
-	    (p->node.flags & PM_DECLAREDNULL) == PM_DECLAREDNULL) {
+	if (printflags & (PRINT_POSIX_READONLY|PRINT_POSIX_EXPORT) &&
+	    p->node.flags & (PM_READONLY|PM_EXPORTED)) {
 	    /*
 	     * Special POSIX rules: show the parameter as readonly/exported
 	     * even though it's unset, but with no value.
@@ -5971,7 +5968,7 @@ printparamnode(HashNode hn, int printflags)
 	}
     }
 
-    if ((printflags & PRINT_NAMEONLY) ||
+    if ((printflags & PRINT_NAMEONLY) || p->node.flags & PM_DECLAREDNULL ||
 	((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..89d6abbba 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) || (v->pm->node.flags & PM_DECLAREDNULL))) ||
 	    (v->flags & VALFLAG_EMPTY))
 	    vunset = 1;
 
diff --git a/Src/zsh.h b/Src/zsh.h
index 6d7f517c6..97d3a142a 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -1929,10 +1929,8 @@ struct tieddata {
 				   made read-only by the user               */
 #define PM_READONLY_SPECIAL (PM_SPECIAL|PM_READONLY|PM_RO_BY_DESIGN)
 #define PM_DONTIMPORT	(1<<22)	/* do not import this variable              */
-#define PM_DECLARED	(1<<22) /* explicitly named with typeset            */
 #define PM_RESTRICTED	(1<<23) /* cannot be changed in restricted mode     */
 #define PM_UNSET	(1<<24)	/* has null value                           */
-#define PM_DECLAREDNULL (PM_DECLARED|PM_UNSET)
 #define PM_REMOVABLE	(1<<25)	/* special can be removed from paramtab     */
 #define PM_AUTOLOAD	(1<<26) /* autoloaded from module                   */
 #define PM_NORESTORE	(1<<27)	/* do not restore value of local special    */
@@ -1942,6 +1940,7 @@ struct tieddata {
 				 */
 #define PM_HASHELEM     (1<<28) /* is a hash-element */
 #define PM_NAMEDDIR     (1<<29) /* has a corresponding nameddirtab entry    */
+#define PM_DECLAREDNULL	(1<<30)	/* declared but null                        */
 
 /* The option string corresponds to the first of the variables above */
 #define TYPESET_OPTSTR "aiEFALRZlurtxUhHTkz"
diff --git a/Test/E03posix.ztst b/Test/E03posix.ztst
index 5e6eddeba..b66782eea 100644
--- a/Test/E03posix.ztst
+++ b/Test/E03posix.ztst
@@ -103,3 +103,11 @@
 >arg1 arg2
 >noktarg1
 >0 0
+
+  f () {
+    local var
+    print ${(t)var}
+  }
+  f
+0:(t) returns correct type
+>scalar-local
-- 
2.30.0.rc1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] declarednull: felipec's approach
  2020-12-23 23:47 [PATCH] declarednull: felipec's approach Felipe Contreras
@ 2020-12-27 18:27 ` Bart Schaefer
  2020-12-27 22:06 ` Bart Schaefer
  2020-12-27 23:04 ` Another push on declarednull branch Bart Schaefer
  2 siblings, 0 replies; 12+ messages in thread
From: Bart Schaefer @ 2020-12-27 18:27 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: zsh-workers

On Wed, Dec 23, 2020 at 3:47 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> This patch merges Bart's approach with my approach, and should be
> applied on top of the branch declarednull.

Thanks Filipe, I'll take a closer look at this.

> In my branch it's actually called PM_NULL because I think semantically
> makes more sense.
[...]
>
> Why would this
>
>   local var
>   var=foobar
>
> remove PM_DELCARED?

I was never especially happy with PM_DECLARED as a name, the real
semantic is "not (yet) assigned" but I couldn't think of a
short/one-word description of that.

> Additionally it's not clear why unset should clear the PM_DECLAREDNULL
> flag.

Because "not (yet) assigned" + "(currently) unset" doesn't mean the
same thing as "(explicitly) unset", in the ksh/bash examples.  Yes, it
would have been enough from the bit value standpoint to clear just
PM_DECLARED (since we're about to re-assert PM_UNSET anyway), but I
thought that made the purpose less clear when reading the code.

> I added a test that shows a discrepancy I found (${(t)var})

Ah, thanks, I forgot to go back and revisit that case.  There's no
ksh/bash equivalent of (t) so I never reached a conclusion on what the
correct behavior should be.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] declarednull: felipec's approach
  2020-12-23 23:47 [PATCH] declarednull: felipec's approach Felipe Contreras
  2020-12-27 18:27 ` Bart Schaefer
@ 2020-12-27 22:06 ` Bart Schaefer
  2020-12-28 20:08   ` Felipe Contreras
  2020-12-27 23:04 ` Another push on declarednull branch Bart Schaefer
  2 siblings, 1 reply; 12+ messages in thread
From: Bart Schaefer @ 2020-12-27 22:06 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: zsh-workers

On Wed, Dec 23, 2020 at 3:47 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> I added a test that shows a discrepancy I found (${(t)var}) but there
> could be many, may more. I only checked one instance of PM_UNSET.

Here's one that your patch gets (I think) wrong:

ubuntu% setopt posixbuiltins
ubuntu% () {
function> readonly foo
function> typeset -p foo
function> print x${(t)foo}x
function> }
xx
ubuntu%

(Note "typeset -p" output nothing.)  On declarednull without your patch:

ubuntu% setopt posixbuiltins
ubuntu% () {
function> readonly foo
function> typeset -p foo
function> print x${(t)foo}x
function> }
typeset -g -r foo
xx
ubuntu%

The typeset output probably comes down to your third hunk in
Src/params.c (printparamnode), although I'm not certain what the
correct fix is, and I don't know what's up with ${(t)foo}.  The
behavior with your patch changes if it's done this way:

ubuntu% () {
function> local foo
function> readonly foo
function> typeset -p foo
function> print x${(t)foo}x
function> }
typeset -r foo
xscalar-local-readonlyx
ubuntu%

More generally, I'm a little concerned by the vunset hunk in
Src/subst.c.  When working on declarednull, I tried a LOT of
variations of fiddling with vunset without getting consistent
behavior. I should have preserved them as test cases, but didn't
because the "make check" suite was not passing everything anyway.
Most involved things like ${+array[x]} for arrays and hashes.

One other thing that has me scratching my head about your patch ... I
can't see any reason why it matters that the bit value is (1<<30), but
if I try, for example, overloading (1<<22) as I did for PM_DECLARED,
the argument lists of shell functions stop working.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Another push on declarednull branch
  2020-12-23 23:47 [PATCH] declarednull: felipec's approach Felipe Contreras
  2020-12-27 18:27 ` Bart Schaefer
  2020-12-27 22:06 ` Bart Schaefer
@ 2020-12-27 23:04 ` Bart Schaefer
  2020-12-28 20:22   ` Felipe Contreras
  2 siblings, 1 reply; 12+ messages in thread
From: Bart Schaefer @ 2020-12-27 23:04 UTC (permalink / raw)
  To: zsh-workers; +Cc: Felipe Contreras

On Wed, Dec 23, 2020 at 3:47 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> In my branch it's actually called PM_NULL because I think semantically
> makes more sense.

Note I'm not rejecting your diff, just fixing things I overlooked in my own.

I OR'd PM_UNSET into PM_DECLAREDNULL because I thought there would be
fewer (and/or less confusing) cases where PM_UNSET had to be ignored
than cases where both PM_UNSET and (new flag) had to be treated as
equivalent, but having found all (hopefully) of the former it's
probably a wash.  Maybe you can still generate a simpler patch.

Alternate names for PM_DECLARED would be welcome.  If I could turn
back time, I might use PM_NOTSET, and then PM_NULL ==
(PM_NOTSET|PM_UNSET).  In fact I already like that enough better that
I'd probably redo it that way before submitting a patch for master.

> I added a test that shows a discrepancy I found (${(t)var})

New push to declarednull branch (tip is now 20e4d07b0) fixes this.
Also added the test from Filipe's patch and another test for readonly
declarations.

Note that workers/47704 (POSIX "readonly -p") hasn't been
committed/pushed anywhere yet, and I don't think we discussed whether
that should do something to "typeset" in ksh emulation.

It may be several days before I can look at this again.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] declarednull: felipec's approach
  2020-12-27 22:06 ` Bart Schaefer
@ 2020-12-28 20:08   ` Felipe Contreras
  2020-12-28 21:00     ` Bart Schaefer
  0 siblings, 1 reply; 12+ messages in thread
From: Felipe Contreras @ 2020-12-28 20:08 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers

On Sun, Dec 27, 2020 at 4:06 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Wed, Dec 23, 2020 at 3:47 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > I added a test that shows a discrepancy I found (${(t)var}) but there
> > could be many, may more. I only checked one instance of PM_UNSET.
>
> Here's one that your patch gets (I think) wrong:
>
> ubuntu% setopt posixbuiltins
> ubuntu% () {
> function> readonly foo
> function> typeset -p foo
> function> print x${(t)foo}x
> function> }
> xx
> ubuntu%
>
> (Note "typeset -p" output nothing.)  On declarednull without your patch:
>
> ubuntu% setopt posixbuiltins
> ubuntu% () {
> function> readonly foo
> function> typeset -p foo
> function> print x${(t)foo}x
> function> }
> typeset -g -r foo
> xx
> ubuntu%
>
> The typeset output probably comes down to your third hunk in
> Src/params.c (printparamnode), although I'm not certain what the
> correct fix is, and I don't know what's up with ${(t)foo}.  The
> behavior with your patch changes if it's done this way:

I see. I would need to check that case.

> One other thing that has me scratching my head about your patch ... I
> can't see any reason why it matters that the bit value is (1<<30), but
> if I try, for example, overloading (1<<22) as I did for PM_DECLARED,
> the argument lists of shell functions stop working.

Because some variables have initially the flag PM_DONTIMPORT (1<<22),
for example IFS, so it's like initially they don't have any value
(i.e. PM_NULL).

Cheers.

-- 
Felipe Contreras


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Another push on declarednull branch
  2020-12-27 23:04 ` Another push on declarednull branch Bart Schaefer
@ 2020-12-28 20:22   ` Felipe Contreras
  2020-12-28 20:53     ` Bart Schaefer
  0 siblings, 1 reply; 12+ messages in thread
From: Felipe Contreras @ 2020-12-28 20:22 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers

On Sun, Dec 27, 2020 at 5:05 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Wed, Dec 23, 2020 at 3:47 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > In my branch it's actually called PM_NULL because I think semantically
> > makes more sense.
>
> Note I'm not rejecting your diff, just fixing things I overlooked in my own.

No worries. My patch was mostly for discussion / exploration purposes.

> I OR'd PM_UNSET into PM_DECLAREDNULL because I thought there would be
> fewer (and/or less confusing) cases where PM_UNSET had to be ignored
> than cases where both PM_UNSET and (new flag) had to be treated as
> equivalent, but having found all (hopefully) of the former it's
> probably a wash.  Maybe you can still generate a simpler patch.
>
> Alternate names for PM_DECLARED would be welcome.  If I could turn
> back time, I might use PM_NOTSET, and then PM_NULL ==
> (PM_NOTSET|PM_UNSET).  In fact I already like that enough better that
> I'd probably redo it that way before submitting a patch for master.

I can recreate the history of the branch as if initially that was the
case (I do it all the time in my patches for git).

However, I'm still not sure if those values make sense.

  typeset var

In this case PM_NULL, PM_NOTSET, and PM_UNSET are true.

  typeset var=''

In this case all of those are false.

  typeset var
  unset var

In this case PM_UNSET is true, but PM_NOTSET is false. Why? No value
was ever assigned.

> > I added a test that shows a discrepancy I found (${(t)var})
>
> New push to declarednull branch (tip is now 20e4d07b0) fixes this.
> Also added the test from Filipe's patch and another test for readonly
> declarations.

It's Felipe BTW.

Cheers.

-- 
Felipe Contreras


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Another push on declarednull branch
  2020-12-28 20:22   ` Felipe Contreras
@ 2020-12-28 20:53     ` Bart Schaefer
  2020-12-28 21:46       ` Felipe Contreras
  0 siblings, 1 reply; 12+ messages in thread
From: Bart Schaefer @ 2020-12-28 20:53 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: zsh-workers

On Mon, Dec 28, 2020 at 12:23 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> On Sun, Dec 27, 2020 at 5:05 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > Alternate names for PM_DECLARED would be welcome.  If I could turn
> > back time, I might use PM_NOTSET, and then PM_NULL ==
> > (PM_NOTSET|PM_UNSET).
>
> However, I'm still not sure if those values make sense.
>
>   typeset var
>   unset var
>
> In this case PM_UNSET is true, but PM_NOTSET is false. Why? No value
> was ever assigned.

Would it make more sense as PM_IMPLICIT, so PM_NULL = (PM_IMPLICIT|PM_UNSET)?

Naming is often a hard problem.  Speaking of which:

> It's Felipe BTW.

Apologies.  As someone who is often Burt/Bert/Bret/Brat, I know the annoyance.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] declarednull: felipec's approach
  2020-12-28 20:08   ` Felipe Contreras
@ 2020-12-28 21:00     ` Bart Schaefer
  2020-12-28 21:58       ` Felipe Contreras
  0 siblings, 1 reply; 12+ messages in thread
From: Bart Schaefer @ 2020-12-28 21:00 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: zsh-workers

On Mon, Dec 28, 2020 at 12:09 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> On Sun, Dec 27, 2020 at 4:06 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > On Wed, Dec 23, 2020 at 3:47 PM Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
> > >
> > One other thing that has me scratching my head about your patch ... I
> > can't see any reason why it matters that the bit value is (1<<30), but
> > if I try, for example, overloading (1<<22) as I did for PM_DECLARED,
> > the argument lists of shell functions stop working.
>
> Because some variables have initially the flag PM_DONTIMPORT (1<<22),
> for example IFS, so it's like initially they don't have any value
> (i.e. PM_NULL).

Yes, but nothing ever looks at PM_DONTIMPORT after setting up the
parameter table, and how did the positional parameters get involved,
and why does that NOT happen on the "raw" declarednull branch?  I
guess it has to be something that's not in your diff but that's
looking for exactly the PM_NULL bit-pattern, such that omitting
PM_UNSET makes that comparison fail.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Another push on declarednull branch
  2020-12-28 20:53     ` Bart Schaefer
@ 2020-12-28 21:46       ` Felipe Contreras
  2020-12-28 22:53         ` Bart Schaefer
  0 siblings, 1 reply; 12+ messages in thread
From: Felipe Contreras @ 2020-12-28 21:46 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers

On Mon, Dec 28, 2020 at 2:54 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Mon, Dec 28, 2020 at 12:23 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > On Sun, Dec 27, 2020 at 5:05 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > >
> > > Alternate names for PM_DECLARED would be welcome.  If I could turn
> > > back time, I might use PM_NOTSET, and then PM_NULL ==
> > > (PM_NOTSET|PM_UNSET).
> >
> > However, I'm still not sure if those values make sense.
> >
> >   typeset var
> >   unset var
> >
> > In this case PM_UNSET is true, but PM_NOTSET is false. Why? No value
> > was ever assigned.
>
> Would it make more sense as PM_IMPLICIT, so PM_NULL = (PM_IMPLICIT|PM_UNSET)?

It's still the same problem isn't it? Why does typeset turn on
PM_IMPLICIT, and unset off?

Moreover, implicit what?

The true meaning is PM_UNSET_BUT_VALID, because the only time this
flag does something is when PM_UNSET is on. So you would have PM_NULL
= (PM_UNSET|PM_UNSET_BUT_VALID).

Then when you check PM_UNSET, you also check !PM_UNSET_BUT_VALID. A
simpler version would be PM_VALID, but when you assign a value,
PM_VALID would be turned off, which doesn't make sense.

I think that's a clear sign the logic is not correct.

It's the other way around:

PM_VALID = !PM_UNSET || PM_NULL

Then PM_NULL is set on typeset, and cleared with unset. Which makes sense.

Then, instead of checking PM_UNSET, you check !PM_VALID, or PM_UNSET
&& !PM_NULL.

> Naming is often a hard problem.  Speaking of which:
>
> > It's Felipe BTW.
>
> Apologies.  As someone who is often Burt/Bert/Bret/Brat, I know the annoyance.

No worries.

-- 
Felipe Contreras


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] declarednull: felipec's approach
  2020-12-28 21:00     ` Bart Schaefer
@ 2020-12-28 21:58       ` Felipe Contreras
  0 siblings, 0 replies; 12+ messages in thread
From: Felipe Contreras @ 2020-12-28 21:58 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers

On Mon, Dec 28, 2020 at 3:00 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Mon, Dec 28, 2020 at 12:09 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > On Sun, Dec 27, 2020 at 4:06 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > >
> > > On Wed, Dec 23, 2020 at 3:47 PM Felipe Contreras
> > > <felipe.contreras@gmail.com> wrote:
> > > >
> > > One other thing that has me scratching my head about your patch ... I
> > > can't see any reason why it matters that the bit value is (1<<30), but
> > > if I try, for example, overloading (1<<22) as I did for PM_DECLARED,
> > > the argument lists of shell functions stop working.
> >
> > Because some variables have initially the flag PM_DONTIMPORT (1<<22),
> > for example IFS, so it's like initially they don't have any value
> > (i.e. PM_NULL).
>
> Yes, but nothing ever looks at PM_DONTIMPORT after setting up the
> parameter table,

Not PM_DONTIMPORT, but PM_NULL (which have the same value).

> and how did the positional parameters get involved,

static initparam argvparam_pm = IPDEF9("", &pparams, NULL, \
PM_ARRAY|PM_SPECIAL|PM_DONTIMPORT);

> and why does that NOT happen on the "raw" declarednull branch?

Because in that branch you never check PM_DECLARED alone; it's always
with PM_UNSET.

> I guess it has to be something that's not in your diff but that's
> looking for exactly the PM_NULL bit-pattern, such that omitting
> PM_UNSET makes that comparison fail.

Yes.

Perhaps clearing the flag after calling dontimport() would make sense.

-- 
Felipe Contreras


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Another push on declarednull branch
  2020-12-28 21:46       ` Felipe Contreras
@ 2020-12-28 22:53         ` Bart Schaefer
  2020-12-29  0:43           ` Felipe Contreras
  0 siblings, 1 reply; 12+ messages in thread
From: Bart Schaefer @ 2020-12-28 22:53 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: zsh-workers

On Mon, Dec 28, 2020 at 1:46 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> On Mon, Dec 28, 2020 at 2:54 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> >
> > Would it make more sense as PM_IMPLICIT, so PM_NULL = (PM_IMPLICIT|PM_UNSET)?
>
> It's still the same problem isn't it? Why does typeset turn on
> PM_IMPLICIT, and unset off?

Typeset turns on PM_IMPLICIT because there's nowhere else that needs to.

Unset turns it off because the variable explicitly ceases to exist;
only its scope remains, if it had one.

> Moreover, implicit what?

Implicit(ly) whatever-other-flags-are-there.

> The true meaning is PM_UNSET_BUT_VALID, because the only time this
> flag does something is when PM_UNSET is on.

I don't think "valid" is a significantly more accurate description.
$thing is "valid" whether or not you've declared "typeset thing".

Anyway, I conceived PM_DECLARED (now for this discussion PM_IMPLICIT)
as having a meaning independent of PM_UNSET; for example
PM_IMPLICIT|PM_INTEGER means that if you assign to the variable, the
value is interpreted as a number.

That was before I realized that "unset foo" forgets everything about
"foo" except its scope.  That is, I began from the position that

  typeset -f foo
  unset foo
  foo=0.3

needed to resurrect foo as a float rather than a string ... but it
doesn't, in bash/ksh.  Hence unset must erase PM_IMPLICIT along with
everything else.

> I think that's a clear sign the logic is not correct.
>
> It's the other way around

Unfortunately given the bitwise implementation that condition is much
more complicated to test for; the reversal is for convenience.  Zsh is
full of bitfield-like booleans that mean nothing unless combined with
other such booleans.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Another push on declarednull branch
  2020-12-28 22:53         ` Bart Schaefer
@ 2020-12-29  0:43           ` Felipe Contreras
  0 siblings, 0 replies; 12+ messages in thread
From: Felipe Contreras @ 2020-12-29  0:43 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: zsh-workers

On Mon, Dec 28, 2020 at 4:53 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Mon, Dec 28, 2020 at 1:46 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > On Mon, Dec 28, 2020 at 2:54 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
> > >
> > > Would it make more sense as PM_IMPLICIT, so PM_NULL = (PM_IMPLICIT|PM_UNSET)?
> >
> > It's still the same problem isn't it? Why does typeset turn on
> > PM_IMPLICIT, and unset off?
>
> Typeset turns on PM_IMPLICIT because there's nowhere else that needs to.

Yes, but why PM_IMPLICIT?

The word "implicit" means unexpressed, or implied. There's nothing
implied about "typeset"; it's expressly declaring a variable.

> Unset turns it off because the variable explicitly ceases to exist;
> only its scope remains, if it had one.

Yes, the variable "ceases to exist", but it's not any less implicit.

> > Moreover, implicit what?
>
> Implicit(ly) whatever-other-flags-are-there.

I presume that doesn't apply to PM_INTEGER, PM_LOCAL, or pretty much
any flag that is not PM_UNSET.

In other words: PM_UNSET is the only flag that can be explicit, or implicit.

> > The true meaning is PM_UNSET_BUT_VALID, because the only time this
> > flag does something is when PM_UNSET is on.
>
> I don't think "valid" is a significantly more accurate description.
> $thing is "valid" whether or not you've declared "typeset thing".

Yes, but that's my point; currently (as in; in master) "valid"
variables are implicitly the ones that are !PM_UNSET, now that concept
is changing.

> Anyway, I conceived PM_DECLARED (now for this discussion PM_IMPLICIT)
> as having a meaning independent of PM_UNSET; for example
> PM_IMPLICIT|PM_INTEGER means that if you assign to the variable, the
> value is interpreted as a number.
>
> That was before I realized that "unset foo" forgets everything about
> "foo" except its scope.  That is, I began from the position that
>
>   typeset -f foo
>   unset foo
>   foo=0.3
>
> needed to resurrect foo as a float rather than a string ... but it
> doesn't, in bash/ksh.  Hence unset must erase PM_IMPLICIT along with
> everything else.

OK. So now it only makes sense with PM_UNSET.

> > I think that's a clear sign the logic is not correct.
> >
> > It's the other way around
>
> Unfortunately given the bitwise implementation that condition is much
> more complicated to test for; the reversal is for convenience.  Zsh is
> full of bitfield-like booleans that mean nothing unless combined with
> other such booleans.

Sure, but PM_NULL can fulfill that purpose perfectly fine (instead of
PM_DECLARED).

-- 
Felipe Contreras


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-12-29  0:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-23 23:47 [PATCH] declarednull: felipec's approach Felipe Contreras
2020-12-27 18:27 ` Bart Schaefer
2020-12-27 22:06 ` Bart Schaefer
2020-12-28 20:08   ` Felipe Contreras
2020-12-28 21:00     ` Bart Schaefer
2020-12-28 21:58       ` Felipe Contreras
2020-12-27 23:04 ` Another push on declarednull branch Bart Schaefer
2020-12-28 20:22   ` Felipe Contreras
2020-12-28 20:53     ` Bart Schaefer
2020-12-28 21:46       ` Felipe Contreras
2020-12-28 22:53         ` Bart Schaefer
2020-12-29  0:43           ` Felipe Contreras

zsh-workers

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.vuxu.org/zsh-workers

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 zsh-workers zsh-workers/ http://inbox.vuxu.org/zsh-workers \
		zsh-workers@zsh.org
	public-inbox-index zsh-workers

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.vuxu.org/vuxu.archive.zsh.workers


code repositories for the project(s) associated with this inbox:

	https://git.vuxu.org/mirror/zsh/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git