Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Pointer variable debugger issues #2176

Closed
willfaught opened this issue Dec 5, 2018 · 6 comments
Closed

Pointer variable debugger issues #2176

willfaught opened this issue Dec 5, 2018 · 6 comments

Comments

@willfaught
Copy link

The fix for #1989 worked for me (see #2164 (comment)), although the dereferenced pointer value is labeled with a single colon, which might be a bug. See the b variable below:

screen shot 2018-12-04 at 11 42 02 pm

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)?

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Dec 6, 2018

@brycekahle, thoughts?

@brycekahle
Copy link
Contributor

although the dereferenced pointer value is labeled with a single colon, which might be a bug

This is how VS Code displays variables. It wants a name and value, but there is no name for the dereferenced value. I mentioned this design choice in the original PR and it was deemed good enough.

The debugger presentation of the pointer type seems incompatible with the other integral types

Incompatible in what way? They are not identical, but AFAIK this issue is about design decisions and doesn't impact functionality in any way.

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

IMHO we should leave the type information in. It may not be obvious from the code what the type is. The debugger is helping here by removing ambiguity. We show type information for other variables as well ([]uint8, bytes.Buffer in the screenshot posted, for example).

@willfaught
Copy link
Author

willfaught commented Dec 6, 2018

This is how VS Code displays variables. It wants a name and value, but there is no name for the dereferenced value. I mentioned this design choice in the original PR and it was deemed good enough.

I'm not sure what value there is in having a separate line for the dereferenced value. Is it possible to omit the dereferenced value and display the fields directly under the pointer? For example, instead of having:

b: <*bytes.Buffer>(0x123)
    : <bytes.Buffer>
        buf: <[]uint8>

would it work to have this instead:

b: <*bytes.Buffer>(0x123)
    buf: <[]uint8>

Incompatible in what way? They are not identical, but AFAIK this issue is about design decisions and doesn't impact functionality in any way.

This is explained by the sentence that follows it about how different cases are displayed:

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)

Pointer values are integers too, just like the int and X types in the example above.

IMHO we should leave the type information in. It may not be obvious from the code what the type is. The debugger is helping here by removing ambiguity. We show type information for other variables as well ([]uint8, bytes.Buffer in the screenshot posted, for example).

OK, so then shouldn't we display i and x above as i: <int>(0) and x: <X>(34) to be consistent? Same for string("foo"), rune('Z'), etc. Your argument seems to apply generally.

@brycekahle
Copy link
Contributor

I'm not sure what value there is in having a separate line for the dereferenced value. Is it possible to omit the dereferenced value and display the fields directly under the pointer?

A trivial amount of value is in showing the unambiguous data relationship. It is definitely possible to display it, as in your 2nd example. This would be more difficult than the original changes I made, but not impossible. I'd be happy to review your PR if you wanted to attempt that change.

This is explained by the sentence that follows it about how different cases are displayed:

Incompatible, for me, indicates some sort of functionality issue. IMHO Displaying pointers differently than simple types, is not an incompatibility.

OK, so then shouldn't we display i and x above as i: <int>(0) and x: <X>(34) to be consistent? Same for string("foo"), rune('Z'), etc

Perhaps, but that is far beyond the scope of the original request in #1989, which was to improve the display of pointer values. I did not change the existing display behavior for other types.

@willfaught
Copy link
Author

A trivial amount of value is in showing the unambiguous data relationship. It is definitely possible to display it, as in your 2nd example. This would be more difficult than the original changes I made, but not impossible. I'd be happy to review your PR if you wanted to attempt that change.

OK, good to know. Thanks.

Incompatible, for me, indicates some sort of functionality issue. IMHO Displaying pointers differently than simple types, is not an incompatibility.

Inconsistent might have been a better word for it. As the issue description said, the feature worked for me, assuming the single colon was working as intended.

Perhaps, but that is far beyond the scope of the original request in #1989, which was to improve the display of pointer values. I did not change the existing display behavior for other types.

That's what this issue is about, aside from the colon question. Since the colon is fine, this issue doesn't have to hold up shipping #1989 as-is, but the part about consistency is still unresolved, it seems.

@ramya-rao-a
Copy link
Contributor

Hello all,

We are in the midst of a repo move, see We are moving section in our readme for more details.

Please subscribe to golang/vscode-go#112 for further updates on this issue.

Thanks for all the support & Happy Coding!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants