debug: issues displaying dereferenced pointer value in the Variable pane

[ramya-rao-a]

From @willfaught in microsoft/vscode-go#2176:

The dereferenced pointer value is labeled with a single colon, which might be a bug. See the b variable below:

The debugger presentation of the pointer type seems incompatible with the other integral types (variables i and x in the example above). int(0) is displayed as i: 0, X(34) (where X is type X int) is displayed as x: 34, but (*int)(0xc0000160f8) is displayed as p: <*int>(0xc0000160f8). Maybe it makes sense to display it as just p: 0xc0000160f8 and let the user infer the type from the code like for the other variable types (and the presence of the drill down menu)?

Please see microsoft/vscode-go#2176 for more discussion on this

[hyangah]

cc @polinasok

A similar issue was discussed recently in #567 but, there users expressed interest in easier inspection of the dereferenced values. My personal preference is to present the type, pointer value, and dereferenced values (if available) rather than switching to p: 0xc0000160f8 for <*int> in the name of consistency.

I agree the name-less : looks strange.

[polinasok]

@hyangah
+1 that we should not be achieving consistency by stripping useful type information.

The current behavior is consistent with what happens with dlv cli:

(dlv) p myInt
12
(dlv) p &myInt
(*int)(0xc000125e58)

This is also consistent with other compound types that have fields, items or elements, such as arrays, maps, interfaces. Those also show type, not value, at the top and an option to expand into the child elements that build up the value.

A pointer is more than just the raw address because it has an underlying type. And without the type, you can't tell the difference between different kinds of pointers just from the numeric address, but you can tell the difference between ints and floats and such from just the value. Scalar types are inherently different, so they don't have to be consistent. But a case can be made that including extra type information into scalars can be useful as well because then you can differentiate between primitive and user-defined types as well as different flavors of ints or floats. Would it at all be helpful to print ints as "int(5)" vs "myInt(5)" vs "int32(5)"?

@willfaught
: does look strange, but it is not a bug. The lefthand side is for the name of a variable or a field. The data that a pointer points to does not have a name, it's empty, so that is what is printed. I guess to make this less confusing we could add a fake name like <dereferenced>:.... or <value>:....

[willfaught]

The current behavior is consistent with what happens with dlv cli

Interesting, good to know!

A pointer is more than just the raw address because it has an underlying type. And without the type, you can't tell the difference between different kinds of pointers just from the numeric address, but you can tell the difference between ints and floats and such from just the value.

True, but you can't tell the difference between an int(1) and an X(1) (type X int, as above) because they both display as 1.

But a case can be made that including extra type information into scalars can be useful as well because then you can differentiate between primitive and user-defined types as well as different flavors of ints or floats. Would it at all be helpful to print ints as "int(5)" vs "myInt(5)" vs "int32(5)"?

Agreed that "int(5)" vs "myInt(5)" vs "int32(5)" would be more helpful. Perhaps the pointer syntax could be changed from <*bytes.Buffer>(0xc0000160f8) to a more Go-like (*bytes.Buffer)(0xc0000160f8), which would be consistent with them.

I guess to make this less confusing we could add a fake name like :.... or :....

The Go spec defines pointer types with:

A pointer type denotes the set of all pointers to variables of a given type, called the base type of the pointer. The value of an uninitialized pointer is nil.

So perhaps "value" is best.

[polinasok]

Perhaps the pointer syntax could be changed from <*bytes.Buffer>(0xc0000160f8) to a more Go-like (*bytes.Buffer)(0xc0000160f8).

This would require adjusting how all other types are displayed as they all use angular brackets to differentiate types from values visually.

[gopherbot]

Change https://golang.org/cl/252979 mentions this issue: src/debugAdapter: present unreadable values correctly

[polinasok]

We are working on a new version of the adapter (please see dlv-dap.md). It inlines values in place of just the type, so you don't even need to expand to view the data in many cases. I believe this addresses some of the issues described here.

[polinasok]

With the switch to dlv-dap adapter, pointers are now printed as *{whatever the underlying type is printed as inlined}. This makes it possible to inspect the dereferenced values without expanding, but pointers to scalars are now displayed without the type.

	legacy	dlv-dap = (dlv) p {var}	(dlv) locals
*int	<*int>(0xc000014080)	*10	(*int)(0xc00010c008)
*myInt	<*main.myInt>(0xc000014088)	*10	(*main.myInt)(0xc00010c010)
*myStruct	<*main.myStruct>(0xc000014090)	*main.myStruct {i: 10}	(*main.myStruct)(0xc00010c018)
*[]int	<*[]int>(0xc00000c030)	*[]int len: 3, cap: 3, [1,2,3]	(*[]int)(0xc000118000)
*interface{}(int)	<*interface {}>(0xc000010240)	*interface {}(int) 10	(*interface {})(0xc000108060)
*interface{}(main.myStruct)	<*interface {}>(0xc000010250)	*interface {}(main.myStruct) {i: 10}	(*interface {})(0xc000108070)

you can't tell the difference between an int(1) and an X(1) (type X int, as above) because they both display as 1.
"int(5)" vs "myInt(5)" vs "int32(5)" would be more helpful

The omitted scalar type can be retrieved by hovering over the variable name.

This does require one-by-one inspection of the variables. We can consider adding type to scalar values and then getting them for free when printing pointers.

For example,

i: int 10
ii: myInt 10
ip: *int 10
ip: *myInt 10

This could be done in dlv's pretty printer shared with cli or in the dap layer.

Perhaps the pointer syntax could be changed from <*bytes.Buffer>(0xc0000160f8) to a more Go-like (*bytes.Buffer)(0xc0000160f8)

We no longer show the pointer address. Unfortunately, unlike dlv cli it cannot be looked up with & either

This might present an issue for some users. We can consider adding the address either inlined (in dlv's pretty printer or dap layer):

ip: (0xc00010c008) *int 10 # if type comes from underlying value
ip: (*int)(0xc00010c008) 10  # if we decide not to upgrade the scalar values with types

or as nested child (in dap layer)

ip: *int 10
    addr: 0xc0000160f8
    : int 10

name-less : looks strange.
to make this less confusing we could add a fake name like "value"

This has come up a couple of times before. If we inline all the necessary information, most people won't click through to see this. If we add address as a child, then wee might need to revisit giving the value its own name placeholder.

ip: *int 10
    addr: 0xc0000160f8
    value: int 10

@aarzilli

[aarzilli]

it's weird that vscode doesn't have a UI to see types and addresses. I guess the javascript bias is still strong. There's another way to go here: add the type to the variable name instead of the value, so you have i int instead of i listed under Locals. The problem there is that type names are, generally, quite long because of the fully qualified package paths (gdlv has a bunch of ugly code in https://github.com/aarzilli/gdlv/blob/master/internal/prettyprint/prettyprint.go#L419 to shorten types, and now that I look at it I think it will be very complicated to adapt it to support generics).

[polinasok]

There is type field in dap Variable, but it is intended to go into hover for each var. No address field though.

Types being too long is the unfortunate reality and has come up before in other contexts, so I would be hesitant to add that to the variable names. Interestingly, it would be consistent with what .NET does:

Although in the past they had the type at the end of the value, which is also an option and would be more compatible with long types:

I will add some c++ snapshots for inspiration tomorrow.

[polinasok]

C++ show the pointer address, but one must hoover for type.

[polinasok]

I think the hover for looking up type is sufficient for now. We can revisit if we hear more from the users. One possibility would be to have a showVariableTypes control to turn types on and off on demand with dlv config like we do for stacktrace view. Missing address appears to be a more pressing issue.

[hyangah]

Personally, I like C++'s way.

Furthermore, if we can move most the type info into the hover, and take it out of the inlined value presentation, that will be consistent and help reducing the cruft caused by the type info. Still type is long, but visible only when people hover over.

IMO most likely, people know the types of the variables they are investigating and filling the screenspace with the type info isn't great.

[willfaught]

@polinasok

With the switch to dlv-dap adapter, pointers are now printed as *{whatever the underlying type is printed as inlined}. This makes it possible to inspect the dereferenced values without expanding, but pointers to scalars are now displayed without the type.

How are pointer values (not the values they reference) inspected now? Do I understand correctly that now for *int it will print *10 instead of anything like 0xc0000160f8? If that's so, I think that's a mistake. We need a way to tell if two pointers point at the same thing. E.g.

var p, q *int
// ...
for p != q {
    // ...
}

When the Locals show

p: 0xc0000160f8
q: 0xc000065c80

we know what the code will do, whereas for

p: *10
q: *10

who knows what will happen? It depends on the pointer values themselves, not the values they reference.

I suggest we omit the types entirely like C++. The type info is in the source code that you're (presumably) looking at as you step through it. It's helpful for debug info to be as concise and readable as possible. Just use the most concise form of the underlying type: 1234 or 0x1234 for numbers and pointers, "abcd" for strings, [1,2,3,4] for arrays and slices, {1,2,3} for structs, {1:2,3:4} for maps, etc. Cut off long representations with an ellipsis (e.g. [1,2,3,4,5,6,8,9…] and {1:2,3:4,5:6,7:8,9:0…}), or let it be horizontally scrollable and "run off" the panel if that's possible and works well. Use drill down menus for attributes and sub-values like slice/array/map/channel len, slice/map/channel cap, referenced values, struct fields, map key/values, etc. Use "value" or "(value)" for the name of the pointer sub-value if it's not nil, use "len" or "(len)" or slice lengths, etc. If someone wants to know a referenced value, they can drill down for it like anything else.