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

dataBreakpointInfo and setDatabreakpointsRequest needs new fields #404

Open
theIDinside opened this issue Jun 14, 2023 · 1 comment
Open
Labels
under-discussion Issue is under discussion for relevance, priority, approach

Comments

@theIDinside
Copy link

theIDinside commented Jun 14, 2023

dataBreakpointInfo request takes the following parameters:

interface DataBreakpointInfoArguments {
  /**
   * Reference to the variable container if the data breakpoint is requested for
   * a child of the container. The `variablesReference` must have been obtained
   * in the current suspended state. See 'Lifetime of Object References' in the
   * Overview section for details.
   */
  variablesReference?: number;

  /**
   * The name of the variable's child to obtain data breakpoint information for.
   * If `variablesReference` isn't specified, this can be an expression.
   */
  name: string;

  /**
   * When `name` is an expression, evaluate it in the scope of this stack frame.
   * If not specified, the expression is evaluated in the global scope. When
   * `variablesReference` is specified, this property has no effect.
   */
  frameId?: number;
}

As we can see, frameId has no effect when variablesReference is passed in.

However, frameId will never have an effect - with or without variablesReference, because setDatabreakpoints can not differentiate between a dataBreakpointInfo that took either a variablesReference or a frameId parameter. Unless the DAP implementer make dataBreakpointsInfo be the actual request that sets the bp, these two requests make no sense (when talking specifically about frameId).

One solution would be to add another field to the response in dataBreakpointInfo that signals what "kind" of dataBreakpointInfo request & response was sent and received. That way, when it's time for setDatabreakpoints request to roll in, the DAP implementation can inspect "should this be set for this frame (because frameId was passed in to the dataBreakpointInfo request, just prior to it) or should it evaluate it as global?".

There is a solution to this, currently for DAP-implementers - and that's to customize (and thus break) the DAP specification when it comes to these two requests, by adding their own fields. But that's not ideal; particularly when the spec does not mention that these requests are runtime specific, which is the case for things like attach and launch requests respectively. The best approach would be if it was standardized in the spec.

This means adding a field that signals if a variablesReference or a frameId was used, to either the dataBreakpointInforesponse or the DataBreakpoint type that later is also sent via setDataBreakpoints request.

That way, when the DAP-implementer receives a setDatabreakpoints request, it can read the frameId and set the watchpoint for that particular frame.

@connor4312
Copy link
Member

because setDatabreakpoints can not differentiate between a dataBreakpointInfo that took either a variablesReference or a frameId parameter

I don't think this is the case. DataBreakpointInfoResponse contains the adapter-defined dataId property, which is a string. The DataBreakpoint in SetDataBreakpoints then requires the dataId. It's up to the adapter to encode any necessary correlation they need into the data ID. For example, they might maintain a random map of UUIDs, or actually encode the frame ID along with other information as a JSON string inside the data ID.

@connor4312 connor4312 added the under-discussion Issue is under discussion for relevance, priority, approach label Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

2 participants