From 2672a8388ac1dbab9de00991ee5804c5b6eb44c0 Mon Sep 17 00:00:00 2001 From: Glen Chung Date: Tue, 19 Nov 2024 15:43:01 -0800 Subject: [PATCH] Support User Defines and Macro References AB Experiment - Depends on https://github.com/microsoft/vscode-cpptools/pull/12979 to merge first. - copilotcppMacroReferenceFilter: regex string to filter macro reference for telemetry. - Telemetry related to user defines and macro references. - Added new trait compilerUserDefines to note the relevant user defines to the editing file. --- Extension/src/LanguageServer/client.ts | 2 + .../src/LanguageServer/copilotProviders.ts | 4 + Extension/src/LanguageServer/lmTool.ts | 33 +++++++- .../tests/copilotProviders.test.ts | 10 ++- .../SingleRootProject/tests/lmTool.test.ts | 84 +++++++++++++++++-- 5 files changed, 123 insertions(+), 10 deletions(-) diff --git a/Extension/src/LanguageServer/client.ts b/Extension/src/LanguageServer/client.ts index c05034e643..ee19fdfed6 100644 --- a/Extension/src/LanguageServer/client.ts +++ b/Extension/src/LanguageServer/client.ts @@ -543,6 +543,8 @@ export interface ChatContextResult { export interface FileContextResult { compilerArguments: string[]; + compilerUserDefines: string[]; + macroReferences: string[]; } export interface ProjectContextResult { diff --git a/Extension/src/LanguageServer/copilotProviders.ts b/Extension/src/LanguageServer/copilotProviders.ts index c51f2b88f7..9943e9fb3e 100644 --- a/Extension/src/LanguageServer/copilotProviders.ts +++ b/Extension/src/LanguageServer/copilotProviders.ts @@ -79,6 +79,10 @@ export async function registerRelatedFilesProvider(): Promise { const compilerArgumentsValue = updatedArguments.join(", "); traits.push({ name: "compilerArguments", value: compilerArgumentsValue, includeInPrompt: true, promptTextOverride: `The compiler arguments include: ${compilerArgumentsValue}.` }); } + if (cppContext.compilerUserDefinesRelevant.length > 0) { + const compilerUserDefinesValue = cppContext.compilerUserDefinesRelevant.join(", "); + traits.push({ name: "compilerUserDefines", value: compilerUserDefinesValue, includeInPrompt: true, promptTextOverride: `These compiler command line user defines may be relevent: ${compilerUserDefinesValue}.` }); + } if (directAsks) { traits.push({ name: "directAsks", value: directAsks, includeInPrompt: true, promptTextOverride: directAsks }); } diff --git a/Extension/src/LanguageServer/lmTool.ts b/Extension/src/LanguageServer/lmTool.ts index 82c54d3135..f6acf44033 100644 --- a/Extension/src/LanguageServer/lmTool.ts +++ b/Extension/src/LanguageServer/lmTool.ts @@ -66,6 +66,7 @@ export interface ProjectContext { targetPlatform: string; targetArchitecture: string; compilerArguments: string[]; + compilerUserDefinesRelevant: string[]; } // Set these values for local testing purpose without involving control tower. @@ -93,6 +94,15 @@ function filterComplierArguments(compiler: string, compilerArguments: string[], return compilerArguments.filter(arg => defaultFilter?.test(arg) || additionalFilter?.test(arg)); } +// We can set up copilotcppMacroReferenceFilter feature flag to filter macro references to learn about +// macro usage distribution, i.e., compiler or platform specific macros, or even the presence of certain macros. +const defaultMacroReferenceFilter: RegExp | undefined = undefined; +function filterMacroReferences(macroReferences: string[], context: { flags: Record }): string[] { + const filter: RegExp | undefined = context.flags.copilotcppMacroReferenceFilter ? new RegExp(context.flags.copilotcppMacroReferenceFilter as string) : undefined; + + return macroReferences.filter(macro => defaultMacroReferenceFilter?.test(macro) || filter?.test(macro)); +} + export async function getProjectContext(context: { flags: Record }, token: vscode.CancellationToken): Promise { const telemetryProperties: Record = {}; try { @@ -110,7 +120,8 @@ export async function getProjectContext(context: { flags: Record 0) { @@ -137,6 +149,23 @@ export async function getProjectContext(context: { flags: Record 0) { + const userDefinesWithoutValue = projectContext.result.fileContext.compilerUserDefines.map(value => value.split('=')[0]); + const userDefinesRelatedToThisFile = userDefinesWithoutValue.filter(value => projectContext.result?.fileContext.macroReferences.includes(value)); + if (userDefinesRelatedToThisFile.length > 0) { + // Don't care the actual name of the user define, just the count that's relevant. + telemetryProperties["compilerUserDefinesRelevantCount"] = userDefinesRelatedToThisFile.length.toString(); + result.compilerUserDefinesRelevant = userDefinesRelatedToThisFile; + } + } + telemetryProperties["macroReferenceCount"] = projectContext.result.fileContext.macroReferences.length.toString(); + if (projectContext.result.fileContext.macroReferences.length > 0) { + const filteredMacroReferences = filterMacroReferences(projectContext.result.fileContext.macroReferences, context); + if (filteredMacroReferences.length > 0) { + telemetryProperties["filteredMacroReferences"] = filteredMacroReferences.join(', '); + } + } return result; } diff --git a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts index f1c1e8908b..a8b48a90e6 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts @@ -138,7 +138,8 @@ describe('copilotProviders Tests', () => { compiler: 'MSVC', targetPlatform: 'Windows', targetArchitecture: 'x64', - compilerArguments: [] + compilerArguments: [], + compilerUserDefinesRelevant: [] }; it('should not provide project context traits when copilotcppTraits flag is false.', async () => { @@ -245,7 +246,8 @@ describe('copilotProviders Tests', () => { compiler: 'MSVC', targetPlatform: 'Windows', targetArchitecture: 'x64', - compilerArguments: ['/std:c++17', '/GR-', '/EHs-c-', '/await'] + compilerArguments: ['/std:c++17', '/GR-', '/EHs-c-', '/await'], + compilerUserDefinesRelevant: ['MY_FOO', 'MY_BAR'] }; it('should provide compiler command line traits if available.', async () => { @@ -266,6 +268,10 @@ describe('copilotProviders Tests', () => { ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.value === '/std:c++17, /GR-, /EHs-c-, /await', 'result.traits should have a compiler arguments trait with value "/std:c++17, /GR-, /EHs-c-, /await"'); ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.includeInPrompt, 'result.traits should have a compiler arguments trait with includeInPrompt true'); ok(result.traits.find((trait) => trait.name === 'compilerArguments')?.promptTextOverride === 'The compiler arguments include: /std:c++17, /GR-, /EHs-c-, /await.', 'result.traits should have a compiler arguments trait with promptTextOverride'); + ok(result.traits.find((trait) => trait.name === 'compilerUserDefines'), 'result.traits should have a compiler user defines trait'); + ok(result.traits.find((trait) => trait.name === 'compilerUserDefines')?.value === 'MY_FOO, MY_BAR', 'result.traits should have a compiler user defines trait with value "MY_FOO, MY_BAR"'); + ok(result.traits.find((trait) => trait.name === 'compilerUserDefines')?.includeInPrompt, 'result.traits should have a compiler user defines trait with includeInPrompt true'); + ok(result.traits.find((trait) => trait.name === 'compilerUserDefines')?.promptTextOverride === 'These compiler command line user defines may be relevent: MY_FOO, MY_BAR.', 'result.traits should have a compiler args trait with promptTextOverride'); ok(!result.traits.find((trait) => trait.name === 'directAsks'), 'result.traits should not have a direct asks trait'); }); diff --git a/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts b/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts index d0a8bc6de4..015d21ea96 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/lmTool.test.ts @@ -179,13 +179,21 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler, context, compilerArguments: compilerArguments, - expectedCompilerArguments + expectedCompilerArguments, + compilerUserDefines, + expectedCompilerUserDefines, + macroReferences, + expectedMacroReferences }: { compiler: string; expectedCompiler: string; context: { flags: Record }; compilerArguments: string[]; expectedCompilerArguments: string[]; + compilerUserDefines: string[]; + expectedCompilerUserDefines: string[]; + macroReferences: string[]; + expectedMacroReferences: string[]; }) => { arrangeProjectContextFromCppTools({ projectContextFromCppTools: { @@ -195,7 +203,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { targetPlatform: 'windows', targetArchitecture: 'x64', fileContext: { - compilerArguments: compilerArguments + compilerArguments: compilerArguments, + compilerUserDefines: compilerUserDefines, + macroReferences: macroReferences } } }); @@ -228,6 +238,22 @@ describe('CppConfigurationLanguageModelTool Tests', () => { "filteredCompilerArguments": expectedCompilerArguments.join(', ') }))); } + ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({ + "compilerUserDefinesCount": compilerUserDefines.length.toString() + }))); + if (expectedCompilerUserDefines.length > 0) { + ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({ + "compilerUserDefinesRelevantCount": expectedCompilerUserDefines.length.toString() + }))); + } + ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({ + 'macroReferenceCount': macroReferences.length.toString() + }))); + if (expectedMacroReferences.length > 0) { + ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({ + 'filteredMacroReferences': expectedMacroReferences.join(', ') + }))); + } ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({ 'time': sinon.match.string }))); @@ -238,6 +264,7 @@ describe('CppConfigurationLanguageModelTool Tests', () => { ok(result.targetPlatform === 'Windows'); ok(result.targetArchitecture === 'x64'); ok(JSON.stringify(result.compilerArguments) === JSON.stringify(expectedCompilerArguments)); + ok(JSON.stringify(result.compilerUserDefinesRelevant) === JSON.stringify(expectedCompilerUserDefines)); }; it('should log telemetry and provide cpp context properly when experimental flags are not defined.', async () => { @@ -246,7 +273,11 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'GCC', context: { flags: {} }, compilerArguments: ['-Wall', '-Werror', '-std=c++20'], - expectedCompilerArguments: [] + expectedCompilerArguments: [], + compilerUserDefines: [], + expectedCompilerUserDefines: [], + macroReferences: [], + expectedMacroReferences: [] }); }); @@ -256,7 +287,39 @@ describe('CppConfigurationLanguageModelTool Tests', () => { expectedCompiler: 'GCC', context: { flags: { copilotcppGccCompilerArgumentFilter: '^-(fno\-exceptions|fno\-rtti)$' } }, compilerArguments: ['-Wall', '-Werror', '-std=c++20', '-fno-exceptions', '-fno-rtti', '-pthread', '-O3', '-funroll-loops'], - expectedCompilerArguments: ['-fno-exceptions', '-fno-rtti'] + expectedCompilerArguments: ['-fno-exceptions', '-fno-rtti'], + compilerUserDefines: [], + expectedCompilerUserDefines: [], + macroReferences: [], + expectedMacroReferences: [] + }); + }); + + it('should honor content-exclusion and provide user defines based on macro references.', async () => { + await testGetProjectContext({ + compiler: 'gcc', + expectedCompiler: 'GCC', + context: { flags: {} }, + compilerArguments: [], + expectedCompilerArguments: [], + compilerUserDefines: ['FOO', 'BAR', 'XYZ'], + expectedCompilerUserDefines: ['FOO', 'XYZ'], + macroReferences: ['FOO', 'XYZ', 'BAZ'], + expectedMacroReferences: [] + }); + }); + + it('should log macro reference telemetry based on copilotcppMacroReferenceFilter.', async () => { + await testGetProjectContext({ + compiler: 'gcc', + expectedCompiler: 'GCC', + context: { flags: { copilotcppMacroReferenceFilter: '^(FOO|XYZ)$' } }, + compilerArguments: [], + expectedCompilerArguments: [], + compilerUserDefines: [], + expectedCompilerUserDefines: [], + macroReferences: ['FOO', 'XYZ', 'BAZ'], + expectedMacroReferences: ['FOO', 'XYZ'] }); }); @@ -272,7 +335,11 @@ describe('CppConfigurationLanguageModelTool Tests', () => { } }, compilerArguments: ['-fno-exceptions', '-fno-rtti'], - expectedCompilerArguments: [] + expectedCompilerArguments: [], + compilerUserDefines: [], + expectedCompilerUserDefines: [], + macroReferences: [], + expectedMacroReferences: [] }); }); @@ -285,7 +352,9 @@ describe('CppConfigurationLanguageModelTool Tests', () => { targetPlatform: 'arduino', targetArchitecture: 'bar', fileContext: { - compilerArguments: [] + compilerArguments: [], + compilerUserDefines: [], + macroReferences: [] } } }); @@ -295,6 +364,8 @@ describe('CppConfigurationLanguageModelTool Tests', () => { ok(languageModelToolTelemetryStub.calledOnce, 'logLanguageModelToolEvent should be called once'); ok(languageModelToolTelemetryStub.calledWithMatch('Completions/tool', sinon.match({ "compilerArgumentCount": '0', + "compilerUserDefinesCount": '0', + "macroReferenceCount": '0', "targetArchitecture": 'bar' }))); ok(result, 'result should not be undefined'); @@ -304,5 +375,6 @@ describe('CppConfigurationLanguageModelTool Tests', () => { ok(result.targetPlatform === ''); ok(result.targetArchitecture === 'bar'); ok(result.compilerArguments.length === 0); + ok(result.compilerUserDefinesRelevant.length === 0); }); });