GNU bug report logs - #76748
31.0.50; `unload-feature' unbinds NEW-ALIAS and BASE-VARIABLE of variable alias

Previous Next

Full log

Message #32 received at 76748 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jens Schmidt <jschmidt4gnu <at> vodafonemail.de>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 76748 <at> debbugs.gnu.org
Subject: Re: bug#76748: 31.0.50; `unload-feature' unbinds NEW-ALIAS and
 BASE-VARIABLE of variable alias
Date: Fri, 14 Mar 2025 19:13:37 -0400

>> You could also choose to keep that function "internal"
>> and not document it in the manual/NEWS (like we did for
>> `internal-make-var-non-special`), mostly justified in case its
>> semantics is delicate/messy/ugly.
>
> I decided to do exactly that - if anybody else except `unload-feature'
> is ever going to need that function, she/he can do the documentation.
> Will add some tests, anyway, when the rest of this has been decided
> upon.

At first, I thought it would be better to make it official, but seeing
the number of details you encountered in the semantics, I think you made
the right call: it's fairly delicate/messy/ugly.

> More questions:
>
> 1. I have a bad feeling about butting in between 1st class citizens
>    makunbound and fmakunbound in data.c, but couldn't find a better
>    other place for that function.

I think I'd put it just after `Fdefvaralias`.

> 2. Before or after switching the variable from an alias to a plain one
>    in `internal-delete-indirect-variable', do I somehow need to clean or
>    zero ...val->alias before overwriting it as ...val->value with
>    "Fset (symbol, Qunbound);"?

No, I think the way you do it is fine.

> 3. defvaralias attaches the new alias to the load history as symbol.
>    One could also attach it as (Fcons (Qalias, new_alias)) and provide a
>    new cl method loadhist-unload-element for that case instead of handling
>    aliases together with regular symbols.

Indeed, we could, but I don't see a strong need for that.

> Finally, `defvaralias' and `define-obsolete-variable-alias' have a
> number of other side-effects, which I have not covered in function
> `internal-delete-indirect-variable'.  Please let me know if I should
> care about any of these:
>
> 4. defvaralias potentially calls
>
>      notify_variable_watchers (new_alias, base_variable, Qdefvaralias, Qnil);

I guess we could add a corresponding "unalias" event, indeed.  Not sure
it's worth the trouble, tho, since your code will already call the
watcher because of the `Fset`: for debug purposes (which is what watchers
are designed for) that should be sufficient.

> 5. defvaralias declares both new alias and base symbol as special.

No need to undo that.

> 6. defvaralias sets
>
>      XSYMBOL (new_alias)->u.s.trapped_write =
>        XSYMBOL (base_variable)->u.s.trapped_write;

We already have a mess here, AFAICT, because `add-variable-watcher`
doesn't keep those two in-sync, so I wouldn't worry.

> 7. defvaralias puts the alias docstring into the variable-documentation
>    property.

I think this does need to be undone.

> 8. define-obsolete-variable-alias copies properties `saved-value',
>    `saved-variable-comment'  from the obsolete to the non-obsolete
>    symbol.

This doesn't need to be undone, I think.

> 9. define-obsolete-variable-alias sets property `byte-obsolete-variable'
>    on the obsolete symbol.

Hmm... I guess in practice it'd be marginally more often useful if we
undo this, but that's just a wild guess.

I think you forgot:

   10. We don't know if the variable was `boundp` before it was turned
       into an alias, so maybe setting it to `Qunbound` is not the right
       way to "undo the defvaralias".

But yeah, I wouldn't worry about it either.

> +DEFUN ("internal-delete-indirect-variable", Finternal_delete_indirect_variable, Sinternal_delete_indirect_variable,
> +       1, 1, 0,
> +       doc: /* Undeclare SYMBOL as variable alias, then unbind it.
> +If SYMBOL is not a variable alias, unbind it like `makunbound' does.
> +Return SYMBOL.  */)
> +  (register Lisp_Object symbol)
> +{
> +  CHECK_SYMBOL (symbol);
> +  if (SYMBOL_CONSTANT_P (symbol))
> +    xsignal1 (Qsetting_constant, symbol);
> +  else if (XSYMBOL (symbol)->u.s.redirect == SYMBOL_VARALIAS)
> +    XSYMBOL (symbol)->u.s.redirect = SYMBOL_PLAINVAL;
> +  Fset (symbol, Qunbound);
> +  return symbol;
> +}

I think I'd signal an error if SYMBOL is not a variable alias (and then
you can get rid of the `SYMBOL_CONSTANT_P` test).



        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.