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

Add "source code reference" to Variable #372

Closed
vogelsgesang opened this issue Feb 10, 2023 · 9 comments
Closed

Add "source code reference" to Variable #372

vogelsgesang opened this issue Feb 10, 2023 · 9 comments
Assignees
Milestone

Comments

@vogelsgesang
Copy link

vogelsgesang commented Feb 10, 2023

TLDR: I would like to have a "go to source location" button in my debugger's variable view. This button would not take me to the source location where the variable was defined, but would take me to the source location associated with the variable's current value. E.g., for C/C++/Rust/Go/Javascript/Typescript... function pointers, it would take me to the place in the source code where that function is defined.

This is not a duplicate of #343. With #343 the source code location would refer to the place of the variable definition, which is different from the place where the variable points to

The problem/use case

Many languages (C, C++, Rust, Go, Python, Javascript, TypeScript, ...) have function pointers or some other type of "function references". The current debugging experience for such function pointers is acceptable but not ideal.
E.g., a C++ function pointer is currently displayed as
Screenshot 2023-02-10 at 15 42 24

If I now want to go to the definition of the function which f points to, I need to read the source location ("at func-ptr-example.cpp:3:14") from the value's description and navigate there manually. This is cumbersome, if I am inspecting many function pointers. Furthermore, for longer function names, the at <location> part of the description is usually hidden, because it doesn't fit into the line.

Besides raw function pointers, C++ and Rust (and probably many other languages) have types which allow introspection at runtime: In C++, there is std::source_location, std::stacktrace. In Rust, there is std::backtrace. Currently, libc++std::source_location (see libc++ implementation) is displayed as

_M_file_name: "/path/to/my/file.cpp"
_M_function_name: "my_function"
_M_line: 353
_M_column: 28

Again, I currently have to manually navigate to the location in that file which is cumbersome if I am inspecting many std::source_locations or a complete std::backtrace.

Preferred solution from a user point of view

As a user of any DAP frontend, I would like to have some "Go to source location" button/hyperlink directly next to my function pointers/std::source_location variables inside the variable view. As a VS Code user, I could imagine e.g. a new button similar to the existing "View Binary Data" (see screenshot). Clicking this new "Go to source code reference" button should take me directly to the correct source location in my editor.

Screenshot 2023-02-10 at 15 49 14

Debug adapters could then associate source code locations with function pointers or variables of any other, arbitrary type. E.g., for C++'s std::function, I could then extend the existing pretty printer from lldb to also expose a source code reference.

Proposed change to Debug Adapter Protocol

Extend the Variable type with a new sourceCodeReference defined as

  /**
   * The source code reference for the variable if the variable refers
   * to a location in a source file. E.g. for function pointers, this should
   * point to the source code which defines the function which the
   * pointer is pointing to.
   * This attribute is only required if the corresponding capability
   * `supportsCodeReference` is true.
   */
  sourceCodeReference?: SourceLocationRange;

with

interface SourceLocationRange {
  /**
   * The path of the source to be shown in the UI.
   * It is only used to locate and load the content of the source if no
   * `sourceReference` is specified (or its value is 0).
   */
  path?: string;

  /**
   * If the value > 0 the contents of the source must be retrieved through the
   * `source` request (even if a path is specified).
   * Since a `sourceReference` is only valid for a session, it can not be used
   * to persist a source.
   * The value should be less than or equal to 2147483647 (2^31-1).
   */
  sourceReference?: number;

  /** E.g., for a function pointer, the begin would point at the beginning of the function definition */
  beginColumn?: number;
  beginLine?: number;
  
  /** E.g., for a function pointer, the end would point at the end of the function definition.
    * We want both begin and end, because multiple functions could be defined in the same
    * line, in particular if using lambda functions instead of full function definitions.
    * Having both `begin` and `end` available, the editor can highlight the correct part of the line.
    */
  endLine?: number;
  endColumn?: number;

  /**
   * The checksums associated with this file.
   */
  checksums?: Checksum[];
}
@vogelsgesang vogelsgesang changed the title Add "source location reference" to Variable Add "source code reference" to Variable Feb 10, 2023
@gregg-miskelly
Copy link
Member

This proposal makes sense to me. Two suggested tweaks:

  • Rename sourceCodeReference to sourceLocation (or similar) since Reference generally had a different meaning in the DAP
  • This should be part of both EvaluateResponse,Variable and probably SetExpressionResponse.

@vogelsgesang
Copy link
Author

Glad to hear that! So what is the usual process to get something added to DAP/Visual Studio Code?
Should I go ahead and draft a Pull Request? Should I first open an issue in https://github.com/microsoft/vscode to see if the VS Code team is willing to extend VS Code to expose this functionality in the UI?

@vogelsgesang
Copy link
Author

On 2nd thought: A similar mechanism might also be useful for the pretty printers of C++ std::fstream or Python file object. Sometimes those paths might also point to directories and not files (e.g., Pythons PurePath)

Not sure if it makes sense to also cover those use cases with the same DAP extensions, though. What do you think?

If this use case should also be covered by the same DAP extension, I think it makes sense to

  • rename sourceCodeReference to fileReference
  • allow byte offsets as an alternative to line/column number. Most languages know at which offset inside the file they are currently reading data, but don't know line/column numbers

@roblourens
Copy link
Member

I think there is definitely interest in this, we discussed something similar in #368. I'll add this to the March milestone and we can discuss more when @connor4312 is back. My only feedback is that I think we should be using the Source type here, your type seems to duplicate it?

I'll point out that for a vscode-specific short term solution, if your debug adapter maps the description to an absolute path + line number, or adds a "fake" metadata child property pointing to the location, then link detection for the variable will work and the user can cmd+click it to go to that location.

@roblourens roblourens added this to the March 2023 milestone Feb 13, 2023
@vogelsgesang
Copy link
Author

I think there is definitely interest in this, we discussed something similar in #368.

Hm... maybe I am misreading that other request... But my understanding from that thread is that it was closed as a duplicate of #343? And #343 was about "link the place the variable was defined" which is different from this request here, i.e. "link the place the value was defined which this variable currently refers to"...

I'll add this to the March milestone and we can discuss more when @connor4312 is back

Thank you! Please let me know if there is anything I can help with

@vogelsgesang
Copy link
Author

that I think we should be using the Source type here, your type seems to duplicate it?

Oh, right. I completely missed Source. Afaik, Source does not include a line number/column number, but that should easily fixable:

interface SourceLocationRange {
  source: Source;

  /** E.g., for a function pointer, the begin would point at the beginning of the function definition */
  line?: number;
  column?: number;
  
  /** E.g., for a function pointer, the end would point at the end of the function definition.
    * We want both begin and end, because multiple functions could be defined in the same
    * line, in particular if using lambda functions instead of full function definitions.
    * Having both `begin` and `end` available, the editor can highlight the correct part of the line.
    */
  endLine?: number;
  endColumn?: number;
}

@roblourens
Copy link
Member

Well in #368 we were talking about the same case (the definition of a function that some variable points at) and I just wasn't drawing a distinction between the two different cases that you are talking about. You make a good point that they are not quite the same.

@vogelsgesang
Copy link
Author

I'll add this to the March milestone and we can discuss more when @connor4312 is back

@roblourens what is the usual process? I would love to be able to make some progress on this. Should I try to draft a PR including all the feedback I received in this commit so far? Do you usually expect to see reference implementations together with a Pull Request against the spec?

@connor4312
Copy link
Member

Sorry for the late followup. Let's merge this into #343

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

No branches or pull requests

4 participants