Re: Compiler warning/error for duplicate variable declaration

[David Favro <lua@...>]

On 12/27/2018 05:24 AM, Domingo Alvarez Duarte wrote:
> Doing several conversions from Lua to LJS https://github.com/mingodad/ljs I 
> found a frequent usage of duplicate variable declarations that make the code 
> hard to understand/port and in some cases it causes bugs that pass unnoticed 
> like this one in luajit 
> https://www.freelists.org/post/luajit/Duplicated-variable-declaration-BUG .

These shadowed declarations are caught by luacheck which prints a warning, 
which I believe can be disabled on a instance-by-instance basis via a 
comment annotation.  That this tool, available for years, is not more widely 
used is a pity: it typically catches these and many other types of smelly code.

Keep in mind that while shadowing names (either in the same scope or an 
inner scope) is generally not a good idea, and no doubt the cause of many 
sleeping bugs in existing codebases, it is currently legal and often 
deliberately used, so it's not always appropriate to treat it as an error or 
even a warning.  Rightly or wrongly, it's not hard to find lots of code like:
local assert = assert;

So, having not examined your patch, if it does not include the ability to 
disable the warning, both globally and for particular instances, it is not 
going to sit well with some programmers.

As an aside, I would note that one frequent cause in my code (and perhaps 
also in dynasm apparently) is that when the rhs of '=' contains multiple 
values (e.g. function-call with multiple return values), and one wants to 
assign to both newly declared local variables and existing locals, upvalues, 
or _ENV globals, the local declaration must be separated from the assignment 
statement which looks awkward and less 'elegant':

local foo;
local function x()
  local bar,foo,gum = y();

This is either a mistake or prone to future bugs, but to avoid the shadowed 
name it becomes:

local foo;
local function x()
  local bar,gum;
  bar,foo,gum = y();

... which doesn't look as nice and seems to decrease the advantages of 
multiple-value assignment.

The various local-by-default and other previous proposals for syntax-based 
scoping might ameliorate this.

-- David