Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of ":" (pointers related) in debugger variables window #567

Closed
emil14 opened this issue Aug 24, 2020 · 6 comments
Closed

Get rid of ":" (pointers related) in debugger variables window #567

emil14 opened this issue Aug 24, 2020 · 6 comments
Labels
Debug Issues related to the debugging functionality of the extension. FrozenDueToAge

Comments

@emil14
Copy link

emil14 commented Aug 24, 2020

Hi! Thank you for the great extension :)

I have a question/request about debugger (variables) visualization. Is it necessary to have a : intermediate level when I explore pointers? Here's the screenshot to make things clear:

Screenshot from 2020-08-24 14-45-56

Here I have to click on data, then click on : and only then I can see the actual value.

Go let's use write x.someMethod() even if x is ponting to something that has someMethod implemented. It would be great to VSCode behave like Go in that sense. Thank you!

@hyangah
Copy link
Contributor

hyangah commented Aug 24, 2020

@emil14 The data behind the "VARIABLES" section is a collection of { name: string, value: string, variablesReference: number } where name is the variable name, value is the value, and variablesReference is the reference handle that is resolved as users expands (e.g. clicking '>' in the UI) further. In UI, VS Code displays each of the variable as name: value form. In your specific example, data is a pointer, that's pointing to an unnamed tree struct and this, in my opinion, is not too far from how Go runtime is seeing the data. When you expanded and inspected the unnamed tree struct, the name is empty and resulted in the : without name.

Much of VS Code's debug UI design was influenced by the javascript/typescript debugging and that may not perfectly fit to Go's data model. I agree that it would be nice if VS Code handles unnamed variables more naturally or we can work around better. But this is not the top priority yet. (It doesn't break any functionality, right?)

For evaluating x.someMethod(), I am currently working on #100 and there is a pending CL under review. I hope it addresses some of what you hoped for. But remember, calling a function or method while not interfering with Go's runtime is hard, and some functions and methods may not be available in compiled binary. So, it's still experimental and may not work as we saw in other languages like JavaScript or python.

@hyangah hyangah added the Question This is a question, rather than an issue report. label Aug 24, 2020
@hyangah hyangah closed this as completed Aug 24, 2020
@polinasok
Copy link
Contributor

@emil14 I see where you are going with the analogy that Go conveniently not forces you to use (*ptr).SomeMethod() or ptr->SomeMethod() like other languages. But in spite of this syntactic sugar, the data is still laid out in memory in a certain way, so that is what the current variable hierarchy reveals. A struct and a pointer to a struct will present differently and with a different number of layers. This is because the dereferenced struct value is a nameless child of the pointer variable. It is its own variable, which is why to get to it you end up going one level deeper. And then you have to get one more level deeper to get to the children, i.e. the fields of the struct variable. Each level issues a new variables request to load more data.

I am currently working on native support for DAP requests in the delve debugger, which we are exposing through a new experimental debug adapter (see #23). As part of this work, I have been investigating how dlv prints variables vs how the vscode UI does it. I identified several cases that could be improved (some already addressed in go-delve/delve#2111, the rest in the works). So while it might not make sense to improve the way pointers are displayed in the current adapter, I can look into improving this in the new version. Let's talk through the details of what this could look like and if that even makes sense.

It looks like dlv itself streamlines this case better. Given the following code:

	myint := 10
	myintptr := &myint
	mystruct := struct {
		i int
		j int
	}{100, 222}
	mystructptr := &mystruct

"locals" command (which is similar to the top-level view in the Variables pane) prints the following:

(dlv) locals
myint = 10
myintptr = (*int)(0xc000065f20)
mystruct = struct { main.i int; main.j int } {i: 100, j: 222}
mystructptr = (*struct { main.i int; main.j int })(0xc000065f18)

while "expanding" each individual variable gives you this:

(dlv) p myintptr
*10
(dlv) p mystructptr
*struct { main.i int; main.j int } {i: 100, j: 222} <=========== fields are inlined
(dlv) p mystruct
struct { main.i int; main.j int } {i: 100, j: 222} <============ looks the same expect for *

So in vscode-go this could look like

<expanded> mystructptr: <*struct { main.i int; main.j int }>(0xc000065f18)
    i: 100
    j: 222

Is that the behavior you are looking for?

In that case, any ideas on how we would we make the scalar case consistent while preserving the type and address info? Or would they look differently (which might be unexpected/misleading)? Currently this presents like so:

<expanded> myintptr: <*int>(0xc0000a3f20)
   : 10

@emil14
Copy link
Author

emil14 commented Aug 25, 2020

@polinasok thank you for detailed explanation :)

So in vscode-go this could look like

mystructptr: <*struct { main.i int; main.j int }>(0xc000065f18)
i: 100
j: 222

Is that the behavior you are looking for?

Yes, this is exactly what I want

In that case, any ideas on how we would we make the scalar case consistent while preserving the type and address info? Or would they look differently (which might be unexpected/misleading)?

Personally I see nothing bad about making them look differently - all I want is productivity and I'm not confused by this difference, the * symbol is enough for me understand that I'm looking at the pointer

@gopherbot
Copy link
Collaborator

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

@polinasok polinasok added the Debug Issues related to the debugging functionality of the extension. label Apr 8, 2021
@polinasok
Copy link
Contributor

polinasok commented Apr 9, 2021

The way we represent variables in the new dlv-dap adapter has evolved since this topic was last discussed. Here is a screenshot. We still have ":", but the value is also inlined.
image

So you will no longer need to always double expand the pointer variable to eyeball its dereferenced contents. I believe that solves the original issue. @emil14 Please confirm if that is the case.

If you do expand the variable, you will still see "<empty name>": <dereferenced pointer value> that can be further expanded for complex types. While I did play with the idea of cutting out that extra intermediate level above, I no longer think it is a good idea. Not only it will require special handling, complicating the code, it will make it impossible to use Add to Watch, Copy As Expression and Copy Value on the dereferenced value.

@polinasok polinasok added FixedInDlvDapOnly and removed Question This is a question, rather than an issue report. labels Apr 9, 2021
@emil14
Copy link
Author

emil14 commented Apr 9, 2021

@polinasok Yes, that solves the issue!

Thank you for noticing me and have a nice day :)

@golang golang locked and limited conversation to collaborators Apr 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Debug Issues related to the debugging functionality of the extension. FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

4 participants