Skip to content

Commit

Permalink
feat: adopt location references to link function locations
Browse files Browse the repository at this point in the history
  • Loading branch information
connor4312 committed Aug 13, 2024
1 parent 6e8a828 commit 272c88d
Show file tree
Hide file tree
Showing 9 changed files with 185 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he

- feat: add basic network view, support experimental networking for node ([#2051](https://github.com/microsoft/vscode-js-debug/issues/2051))
- feat: support "debug url" in terminals created through the `node-terminal` launch type ([#2049](https://github.com/microsoft/vscode-js-debug/issues/2049))
- feat: adopt location references to link function locations
- fix: hover evaluation incorrectly showing undefined ([vscode#221503](https://github.com/microsoft/vscode/issues/221503))

## v1.92 (July 2024)
Expand Down
18 changes: 18 additions & 0 deletions src/adapter/debugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export class DebugAdapter implements IDisposable {
this.dap.on('toggleSkipFileStatus', params => this._toggleSkipFileStatus(params));
this.dap.on('toggleSkipFileStatus', params => this._toggleSkipFileStatus(params));
this.dap.on('prettyPrintSource', params => this._prettyPrintSource(params));
this.dap.on('locations', params => this._onLocations(params));
this.dap.on('revealPage', () => this._withThread(thread => thread.revealPage()));
this.dap.on('getPerformance', () =>
this._withThread(thread => performanceProvider.retrieve(thread.cdp())),
Expand Down Expand Up @@ -374,6 +375,21 @@ export class DebugAdapter implements IDisposable {
return undefined;
}

async _onLocations(params: Dap.LocationsParams): Promise<Dap.LocationsResult> {
const variableStore = this.findVariableStore(v => v.hasVariable(params.locationReference));
if (!variableStore || !this._thread) throw errors.locationNotFound();
const location = await variableStore.getLocations(params.locationReference);
const uiLocation = await this._thread.rawLocationToUiLocationWithWaiting(
this._thread.rawLocation(location),
);
if (!uiLocation) throw errors.locationNotFound();
return {
source: await uiLocation.source.toDap(),
line: uiLocation.lineNumber,
column: uiLocation.columnNumber,
};
}

async _onVariables(params: Dap.VariablesParams): Promise<Dap.VariablesResult> {
const variableStore = this.findVariableStore(v => v.hasVariable(params.variablesReference));
return { variables: (await variableStore?.getVariables(params)) ?? [] };
Expand Down Expand Up @@ -425,6 +441,8 @@ export class DebugAdapter implements IDisposable {
namedVariables: r.namedVariables,
presentationHint: r.presentationHint,
type: r.type,
memoryReference: r.memoryReference,
valueLocationReference: r.valueLocationReference,
};
}

Expand Down
1 change: 1 addition & 0 deletions src/adapter/threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ export class Thread implements IVariableStoreLocationProvider {
namedVariables: variable.namedVariables,
indexedVariables: variable.indexedVariables,
memoryReference: variable.memoryReference,
valueLocationReference: variable.valueLocationReference,
};
}

Expand Down
36 changes: 34 additions & 2 deletions src/adapter/variableStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ class VariableContext {
}

if (object.objectId) {
if (object.subtype === 'map' || object.subtype === 'set') {
if (object.type === 'function') {
return this.createVariable(FunctionVariable, ctx, object, customStringRepr);
} else if (object.subtype === 'map' || object.subtype === 'set') {
return this.createVariable(SetOrMapVariable, ctx, object, customStringRepr);
} else if (!objectPreview.subtypesWithoutPreview.has(object.subtype)) {
return this.createVariable(ObjectVariable, ctx, object, customStringRepr);
Expand Down Expand Up @@ -707,7 +709,7 @@ class StacktraceOutputVariable extends Variable {
}

class FunctionLocationVariable extends Variable {
private readonly location: Cdp.Debugger.Location;
public readonly location: Cdp.Debugger.Location;

constructor(context: VariableContext, remoteObject: Cdp.Runtime.RemoteObject) {
super(context, remoteObject);
Expand Down Expand Up @@ -845,6 +847,20 @@ class ObjectVariable extends Variable implements IMemoryReadable {
}
}

class FunctionVariable extends ObjectVariable {
private readonly baseChildren = once(() => super.getChildren({ variablesReference: this.id }));

public override async toDap(previewContext: PreviewContextType): Promise<Dap.Variable> {
const [dap, children] = await Promise.all([super.toDap(previewContext), this.baseChildren()]);

if (children.some(c => c instanceof FunctionLocationVariable)) {
dap.valueLocationReference = this.id;
}

return dap;
}
}

const entriesVariableName = '[[Entries]]';

class SetOrMapVariable extends ObjectVariable {
Expand Down Expand Up @@ -1435,6 +1451,22 @@ export class VariableStore {
.map(v => v.dap);
}

public async getLocations(variablesReference: number): Promise<Cdp.Debugger.Location> {
const container = this.vars.get(variablesReference);
if (!container) {
throw errors.locationNotFound();
}

// note: is actually free because the container will have cached its children
const children = await container.getChildren({ variablesReference });
const locationVar = children.find(isInstanceOf(FunctionLocationVariable));
if (!locationVar) {
throw errors.locationNotFound();
}

return locationVar.location;
}

/** Sets a variable */
public async setVariable(params: Dap.SetVariableParams): Promise<Dap.SetVariableResult> {
const container = this.vars.get(params.variablesReference);
Expand Down
95 changes: 94 additions & 1 deletion src/dap/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,18 @@ export namespace Dap {
*/
disassembleRequest(params: DisassembleParams): Promise<DisassembleResult>;

/**
* Looks up information about a location reference previously returned by the debug adapter.
*/
on(
request: 'locations',
handler: (params: LocationsParams) => Promise<LocationsResult | Error>,
): () => void;
/**
* Looks up information about a location reference previously returned by the debug adapter.
*/
locationsRequest(params: LocationsParams): Promise<LocationsResult>;

/**
* Sets the enabled custom breakpoints.
*/
Expand Down Expand Up @@ -1670,6 +1682,11 @@ export namespace Dap {
*/
disassemble(params: DisassembleParams): Promise<DisassembleResult>;

/**
* Looks up information about a location reference previously returned by the debug adapter.
*/
locations(params: LocationsParams): Promise<LocationsResult>;

/**
* Sets the enabled custom breakpoints.
*/
Expand Down Expand Up @@ -2328,6 +2345,13 @@ export namespace Dap {
* This attribute may be returned by a debug adapter if corresponding capability `supportsMemoryReferences` is true.
*/
memoryReference?: string;

/**
* A reference that allows the client to request the location where the returned value is declared. For example, if a function pointer is returned, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location.
*
* This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details.
*/
valueLocationReference?: integer;
}

export interface EvaluationOptionsParams {
Expand Down Expand Up @@ -2833,6 +2857,40 @@ export namespace Dap {
sources: Source[];
}

export interface LocationsParams {
/**
* Location reference to resolve.
*/
locationReference: integer;
}

export interface LocationsResult {
/**
* The source containing the location; either `source.path` or `source.sourceReference` must be specified.
*/
source: Source;

/**
* The line number of the location. The client capability `linesStartAt1` determines whether it is 0- or 1-based.
*/
line: integer;

/**
* Position of the location within the `line`. It is measured in UTF-16 code units and the client capability `columnsStartAt1` determines whether it is 0- or 1-based. If no column is given, the first position in the start line is assumed.
*/
column?: integer;

/**
* End line of the location, present if the location refers to a range. The client capability `linesStartAt1` determines whether it is 0- or 1-based.
*/
endLine?: integer;

/**
* End position of the location within `endLine`, present if the location refers to a range. It is measured in UTF-16 code units and the client capability `columnsStartAt1` determines whether it is 0- or 1-based.
*/
endColumn?: integer;
}

export interface LongPredictionEventParams {}

export interface MemoryEventParams {
Expand Down Expand Up @@ -2982,6 +3040,13 @@ export namespace Dap {
* Additional data to report. For the `telemetry` category the data is sent to telemetry, for the other categories the data is shown in JSON format.
*/
data?: any[] | boolean | integer | null | number | object | string;

/**
* A reference that allows the client to request the location where the new value is declared. For example, if the logged value is function pointer, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location.
*
* This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details.
*/
locationReference?: integer;
}

export interface PauseParams {
Expand Down Expand Up @@ -3474,6 +3539,13 @@ export namespace Dap {
* This attribute may be returned by a debug adapter if corresponding capability `supportsMemoryReferences` is true.
*/
memoryReference?: string;

/**
* A reference that allows the client to request the location where the new value is declared. For example, if the new value is function pointer, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location.
*
* This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details.
*/
valueLocationReference?: integer;
}

export interface SetFunctionBreakpointsParams {
Expand Down Expand Up @@ -3574,6 +3646,13 @@ export namespace Dap {
* This attribute may be returned by a debug adapter if corresponding capability `supportsMemoryReferences` is true.
*/
memoryReference?: string;

/**
* A reference that allows the client to request the location where the new value is declared. For example, if the new value is function pointer, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location.
*
* This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details.
*/
valueLocationReference?: integer;
}

export interface SourceParams {
Expand Down Expand Up @@ -4019,6 +4098,20 @@ export namespace Dap {
* This attribute may be returned by a debug adapter if corresponding capability `supportsMemoryReferences` is true.
*/
memoryReference?: string;

/**
* A reference that allows the client to request the location where the variable is declared. This should be present only if the adapter is likely to be able to resolve the location.
*
* This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details.
*/
declarationLocationReference?: integer;

/**
* A reference that allows the client to request the location where the variable's value is declared. For example, if the variable contains a function pointer, the adapter may be able to look up the function's location. This should be present only if the adapter is likely to be able to resolve the location.
*
* This reference shares the same lifetime as the `variablesReference`. See 'Lifetime of Object References' in the Overview section for details.
*/
valueLocationReference?: integer;
}

/**
Expand Down Expand Up @@ -4112,7 +4205,7 @@ export namespace Dap {
endColumn?: integer;

/**
* Indicates whether this frame can be restarted with the `restart` request. Clients should only use this if the debug adapter supports the `restart` request and the corresponding capability `supportsRestartRequest` is true. If a debug adapter has this capability, then `canRestart` defaults to `true` if the property is absent.
* Indicates whether this frame can be restarted with the `restartFrame` request. Clients should only use this if the debug adapter supports the `restart` request and the corresponding capability `supportsRestartFrame` is true. If a debug adapter has this capability, then `canRestart` defaults to `true` if the property is absent.
*/
canRestart?: boolean;

Expand Down
3 changes: 3 additions & 0 deletions src/dap/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ export const noUwpPipeFound = () =>
),
);

export const locationNotFound = () =>
createUserError(l10n.t('Could not find a location for the variable'));

/**
* Returns if the value looks like a DAP error.
*/
Expand Down
2 changes: 2 additions & 0 deletions src/dap/telemetryClassification.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ interface IDAPOperationClassification {
'!writememory.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' };
disassemble: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' };
'!disassemble.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' };
locations: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' };
'!locations.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' };
setcustombreakpoints: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' };
'!setcustombreakpoints.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' };
prettyprintsource: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' };
Expand Down
23 changes: 23 additions & 0 deletions src/test/evaluate/evaluate-supports-location-lookup.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@

> result: ƒ printArr(arr) {
for (const num of arr) {
console.log(plusTwo(num));
}
}
> arguments: (...)
> caller: (...)
length: 1
name: 'printArr'
> prototype: {constructor: ƒ}
[[FunctionLocation]]: @ ${workspaceFolder}/web/basic.ts:5
> [[Prototype]]: ƒ ()
> [[Scopes]]: Scopes[1]
{
column : 18
line : 5
source : {
name : basic.ts
path : ${workspaceFolder}/web/basic.ts
sourceReference : <number>
}
}
9 changes: 9 additions & 0 deletions src/test/evaluate/evaluate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ describe('evaluate', () => {
p.assertLog();
});

itIntegrates('supports location lookup', async ({ r }) => {
const p = await r.launchUrlAndLoad('basic.html');
const fn = await p.logger.evaluateAndLog('printArr');
expect(fn.valueLocationReference).to.be.greaterThan(0);
const location = await p.dap.locations({ locationReference: fn.valueLocationReference! });
p.log(location);
p.assertLog();
});

itIntegrates('repl', async ({ r }) => {
const p = await r.launchUrlAndLoad('index.html');

Expand Down

0 comments on commit 272c88d

Please sign in to comment.