Non-unique variable names within a container

[justarandomgeek]

The Variables type does not specify that its name should be unique within a cotainer, but it seems that DAP assumes that they are in several places:

• At least some implementations fail to list repeated names in a Variables response. (Related Debug Variables View does not show multiple variables from Debug Adapter when names are not unique vscode#107506)
• SetVariables request takes a name argument, which produces ambiguity (and leaves some vars inaccessible) if there are multiple variables of that name
• I'm not using Data Breakpoints yet, but DataBreakpointInfo also seems to use ref+name to identify a variable, with the same problems as SetVariables

This is causing some problems for me, because in Lua it's perfectly valid to do something like this:

local function a()
  local a = 1
  local a = 2
  local a = 3
  local a = 4
  return
end

or

local function a()
  local a = 1
  do
    local a = 2
    print(a) -- uses the second a
  end
  print(a) -- uses the first a again
end

and I've had some complaints from users about getting "the wrong" version of redefined locals.

It seems like the protocol wants me to make a new Scope at every block or something like that, but i don't get enough information from the Lua runtime to do that, and in any case it wouldn't work since redefining within a block is legal anyway.

[justarandomgeek]

Minor follow up: I ended up renaming the affected shadow variables to use otherwise illegal identifier names so that they can be differentiated in Variables results and related requests. It should probably at least be clarified if uniqueness is indeed a protocol requirement.

[weinand]

@justarandomgeek what is the semantic of your first example?
Is a debugger really expected to show four "a" variables without a way to distinguishing them?
Is there a way to qualify the different "a"s, e.g. when I want to add them all in an assignment to a "b":

local b = a + a + a + a

[justarandomgeek]

Yes, I would expect a debugger to show all four of the as (and my debugger now does, with workaround). There's no way for the language to access the shadowed ones normally (only debug libraries) at that point since the scopes all end at the same time (maybe a bad example...), but they still exist as local variables in registers in the Lua VM. My current workaround is to show the shadowed ones in variables view as a<1> a<2> a<3> with their register number (since angle brackets like that aren't legal in Lua, so I don't accidentally replace a real name). The second example is probably clearer since the outer a does become accessible again once the inner one ends - but both exist during the inner block.

There's also a tangentially related situation I occasionally see (but less often) when expanding tables because Lua allows any value to be the key to a table so you might end up with a table whose keys are all empty tables that just get called {} or generally a set of tables that all stringify the same (like local foo = {[{}] = true, [{}] = false, [{}] = "foo"}), and since I don't have access to table addresses to disambiguate them I have nothing useful to attach to these for unique names. However, due to some quirks of the Factorio modding environment, table with tables as keys are much less common than just redefining locals.

[weinand]

yes, the variable name must be unique within its container and DAP clients rely on this.
@connor4312 We should document this in the "Variable.name" property.

Since DAP clients make no other assumptions about variable names, debug adapters are free to create "synthetic"
names like "a<1>" in order to handle cases like the one at hand.

[connor4312]

yes, the variable name must be unique within its container and DAP clients rely on this.

I'm curious why this needs to be the case. From a quick scan of DAP, the variable name is only reused in two places, the DataBreakpointInfoArguments and SetVariableArguments. In both cases, the name is used as supplemental or alternative information to a variablesReference.

That said I still question the utility of the request as presented. As a developer it would be more helpful to have different names like a[line=123] then four ambiguous a's in my variables view.

[roblourens]

But having a variablesReference implies that the variable has children. I think that assuming unique names (for variables that don't have variablesReference) is reasonable