[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: functions can fully unset local vars in other scopes

From: Martin D Kealey
Subject: Re: functions can fully unset local vars in other scopes
Date: Sat, 30 Jul 2022 11:47:01 +1000

On Fri, 29 Jul 2022 at 18:51, Kerin Millar <kfm@plushkava.net> wrote:

> 5.0 added a localvar_unset option that impedes the behaviour of 'popping'
> from the outermost scope.
> $ bash -c 'x=foo; f1() { local x=bar; f2; x=baz; }; f2() { unset x;
> declare -p x; }; f1; declare -p x'
> declare -- x="foo"
> declare -- x="baz"
> $ bash -O localvar_unset -c 'x=foo; f1() { local x=bar; f2; x=baz; }; f2()
> { unset x; declare -p x; }; f1; declare -p x'
> declare -- x
> declare -- x="foo"

Whilst I have serious misgivings about the old behaviour, I find this
approach *deeply*  unsettling, introducing yet more action at a distance:
modifying the behaviour of *existing* code by changing a global option that
didn't even exist when the code was written. Whilst it's possible to
localize the effect of shopt so it doesn't leak out, that's an extra step,
making it likely to be overlooked.

I'll grant that this new shopt is significantly better than simply changing
the default behaviour and then requiring compat44 to get the old behaviour
back (*1), and that version control is "hard", but adding yet another
global setting seems to be going in entirely the wrong direction.

I'm aware that there are historical choices that later turned out to be
suboptimal, but it seems like the better way to fix them is not to
unwittingly change the behaviour of existing code, but rather to introduce
new commands or options to get the new behaviours (*2).

What consideration was given to creating “local --unset *VAR*” or “unset
--local *VAR*” or some other equivalent that doesn't overload any existing
command form?


*1: To my mind, shopt compatNN is broken in at least 4 ways:
 (a) the old behaviour isn't the default (new behaviour should only apply
when a script asks for it; see Python or Perl or, well, pretty much any
mature language that has a stable ecosystem of user-contributed modules);
 (b) there's no way to say "this script is written to work on the CURRENT
version of Bash*";*
 (c) it's too coarse, smushing unrelated changes into one setting;
 (d) it's global, so you can't have different behaviour in different files
or different functions (unless you're prepared to play ridiculous games to
localize the effects).
Bash already changes its behaviour when called as "sh" vs "bash"; it
wouldn't be a huge leap to make any future default "compat" changes
contingent on being called "bash6" - or simply install separate binaries
with defaults.

*2: Adding a new global setting just adds yet another unreasonably
difficulty when sourcing files maintained by multiple authors into one
shell script program. If there's a real case to be made that some old
behaviour is *necessarily* and *always* wrong, then there should be a
separate mechanism for enforcing a change, and it should be automatically
scoped to the current file or function. That said, I'm pleased that
localvar_unset is off by default, because thinking that some existing
behaviour is *always* wrong is itself almost certainly wrong.

Case in point: Bash v4.4 disabled "break" from inside a function called
within a loop, which broke a bunch of rare-but-legitimate use cases,
because nobody making the decision could imagine calling a function
"break_if_match" whose obvious *raison d'être* was flow control.

And no, I'm not about to enter into a debate about other ways this code
could have been written; the point is, it was working fine in Bash v4.3,
without any bugs (at least, none relating to effective flow control) and
then out of the blue it stopped working. Changing Bash so that existing
working scripts stop working is *not* reasonable. Expecting every
maintainer of every shell script in existence to be subscribed to this
mailing list so that they can be forewarned of upcoming changes isn't just
unreasonable, it's utterly ridiculous.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]