From a1fde0643550f7ac4c9b6915939f500b2234549d Mon Sep 17 00:00:00 2001 From: richzwart Date: Fri, 5 Jan 2024 21:07:15 -0500 Subject: [PATCH 1/7] Revert "Revert "Fixed autoinstantiation tripping on brackets and unpacked dimensions"" This reverts commit deab5a9fe51d6db9e40bafc4cf0e00f3a5104c63. --- src/providers/ModuleInstantiator.ts | 63 +++++++++++++++---- src/test/ModuleInstantiator.test.ts | 40 ++++++++++++ .../test-files/ModuleInstantiator.test.1.v | 56 +++++++++++++++++ .../test-files/ModuleInstantiator.test.2.v | 30 +++++++++ 4 files changed, 176 insertions(+), 13 deletions(-) diff --git a/src/providers/ModuleInstantiator.ts b/src/providers/ModuleInstantiator.ts index 64f6504..b4c753d 100644 --- a/src/providers/ModuleInstantiator.ts +++ b/src/providers/ModuleInstantiator.ts @@ -285,6 +285,8 @@ function cleanUpContainer(container: string): string { container = container.replace(/=/g, ' = '); container = container.replace(/\(/g, ' ( '); container = container.replace(/\)/g, ' ) '); + container = container.replace(/\[/g, ' [ '); + container = container.replace(/\]/g, ' ] '); container = container.replace(/\/\//g, ' // '); container = container.replace(/\/\*/g, ' /* '); @@ -326,7 +328,7 @@ function findMaxLength(container: string, moduleIsParameterized: boolean): numbe let lastPort: string = undefined; // eslint-disable-line no-undef-init let lastParameter: string = undefined; // eslint-disable-line no-undef-init let passedEqualSign = false; - + let numBracketPast = 0; let state = ProcessingState.INITIAL; for (let i = 0; i < keys.length; i++) { @@ -351,7 +353,13 @@ function findMaxLength(container: string, moduleIsParameterized: boolean): numbe } } else if (state === ProcessingState.PARAMETERS) { if (keys[i] === ')') { - state = ProcessingState.PORTS; + if (numBracketPast === 0) { + state = ProcessingState.PORTS; + } else { + numBracketPast -= 1; + } + } else if (keys[i] === '(') { + numBracketPast += 1; } else if (keys[i] === ',' && lastParameter) { maxLength = Math.max(lastParameter.length, maxLength); lastParameter = undefined; @@ -369,13 +377,19 @@ function findMaxLength(container: string, moduleIsParameterized: boolean): numbe lastParameter = undefined; } - if (keys[i] === ')') { + if (keys[i] === ')' && numBracketPast == 0) { state = ProcessingState.COMPLETE; } else if (keys[i] === ',' && lastPort) { maxLength = Math.max(lastPort.length, maxLength); lastPort = undefined; + } else if (keys[i] === '[') { + numBracketPast += 1; + } else if (keys[i] === ']') { + numBracketPast -= 1; } else if (!isPortSymbol(keys[i]) && !isEmptyKey(keys[i])) { - lastPort = keys[i].trim(); + if (numBracketPast == 0) { + lastPort = keys[i].trim(); + } } } @@ -416,14 +430,22 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized } const output = []; - const keys = container.split(' '); + // Filtering out keys beforehand makes it slightly less time consuming to debug + const keys = container.split(' ').filter((el) => { + if (isEmptyKey(el)) { + return false; + } else { + return true; + } + }); // This is a bit funky, but the for loop below actually checks if these varaibles // are undefined so it is important that they are initialized as such let lastPort: string = undefined; // eslint-disable-line no-undef-init let lastParameter: string = undefined; // eslint-disable-line no-undef-init - let lastParameterDefault: string = undefined; // eslint-disable-line no-undef-init + let parameterDefault = []; + let numBracketPast = 0; let passedEqualSign = false; let state = ProcessingState.INITIAL; @@ -450,14 +472,19 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized } } else if (state === ProcessingState.PARAMETERS) { if (keys[i] === ')') { - state = ProcessingState.PORTS; + if (numBracketPast === 0) { + state = ProcessingState.PORTS; + } else { + numBracketPast -= 1; + parameterDefault.push(keys[i].trim()); + } } else if (keys[i] === ',' && lastParameter) { // Set with default value if it exists if (passedEqualSign) { output.push( `${padding}.${lastParameter}${' '.repeat(maxLength - lastParameter.length)}${' '.repeat(4)}(` ); - output.push(`${lastParameterDefault})`); + output.push(`${parameterDefault.join('')})`); // This will butcher default values for strings with spaces included, sorry. passedEqualSign = false; } else { @@ -472,9 +499,13 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized lastParameter = undefined; } else if (keys[i] === '=') { passedEqualSign = true; + parameterDefault = []; } else if (!isParameterSymbol(keys[i]) && !isEmptyKey(keys[i])) { if (passedEqualSign) { - lastParameterDefault = keys[i].trim(); + if (keys[i] === '(') { + numBracketPast += 1; + } + parameterDefault.push(keys[i].trim()); } else { lastParameter = keys[i].trim(); } @@ -486,7 +517,7 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized output.push( `${padding}.${lastParameter}${' '.repeat(maxLength - lastParameter.length)}${' '.repeat(4)}(` ); - output.push(`${lastParameterDefault})\n`); + output.push(`${parameterDefault.join('')})\n`); passedEqualSign = false; } else { @@ -500,7 +531,7 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized lastParameter = undefined; } - if (keys[i] === ')') { + if (keys[i] === ')' && numBracketPast == 0) { state = ProcessingState.COMPLETE; } else if (keys[i] === ',' && lastPort) { output.push( @@ -510,7 +541,13 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized lastPort = undefined; } else if (!isPortSymbol(keys[i]) && !isEmptyKey(keys[i])) { - lastPort = keys[i].trim(); + if (keys[i] === '[') { + numBracketPast += 1; + } else if (keys[i] === ']') { + numBracketPast -= 1; + } else if (numBracketPast == 0) { + lastPort = keys[i].trim(); + } } } @@ -522,7 +559,7 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized output.push( `${padding}.${lastParameter}${' '.repeat(maxLength - lastParameter.length)}${' '.repeat(4)}(` ); - output.push(`${lastParameterDefault})\n`); + output.push(`${parameterDefault.join('')})\n`); passedEqualSign = false; } else { diff --git a/src/test/ModuleInstantiator.test.ts b/src/test/ModuleInstantiator.test.ts index 7f79375..3b27910 100644 --- a/src/test/ModuleInstantiator.test.ts +++ b/src/test/ModuleInstantiator.test.ts @@ -199,6 +199,46 @@ suite('ModuleInstantiator Tests', () => { compareInstantiation('golden', container, instance); }); + + test('test #8: formatInstance with unpacked dimensions and brackets in ports', async () => { + let uri = vscode.Uri.file(path.join(__dirname, testFolderLocation, 'test-files', 'ModuleInstantiator.test.1.v')); // prettier-ignore + let document = await vscode.workspace.openTextDocument(uri); + + let fullRange = null; + // Range of the module in the document + fullRange = new vscode.Range(new vscode.Position(301, 6), new vscode.Position(325, 0)); + + let container = document.getText(fullRange).replace(/^\s+|\s+$/g, ''); + + uri = vscode.Uri.file(path.join(__dirname, testFolderLocation, 'test-files', 'ModuleInstantiator.test.2.v')); + document = await vscode.workspace.openTextDocument(uri); + + fullRange = new vscode.Range(new vscode.Position(138, 0), new vscode.Position(147, 0)); + + let instance = document.getText(fullRange); + + compareInstantiation('arrer', container, instance); + }); + + test('test #9: formatInstance with brackets in parameter default value', async () => { + let uri = vscode.Uri.file(path.join(__dirname, testFolderLocation, 'test-files', 'ModuleInstantiator.test.1.v')); // prettier-ignore + let document = await vscode.workspace.openTextDocument(uri); + + let fullRange = null; + // Range of the module in the document + fullRange = new vscode.Range(new vscode.Position(152, 6), new vscode.Position(164, 0)); + + let container = document.getText(fullRange).replace(/^\s+|\s+$/g, ''); + + uri = vscode.Uri.file(path.join(__dirname, testFolderLocation, 'test-files', 'ModuleInstantiator.test.2.v')); + document = await vscode.workspace.openTextDocument(uri); + + fullRange = new vscode.Range(new vscode.Position(152, 0), new vscode.Position(164, 0)); + + let instance = document.getText(fullRange); + + compareInstantiation('azzer', container, instance); + }); }); function compareInstantiation(instance_name, container_name, expected): void { diff --git a/src/test/test-files/ModuleInstantiator.test.1.v b/src/test/test-files/ModuleInstantiator.test.1.v index 37a5384..0296711 100644 --- a/src/test/test-files/ModuleInstantiator.test.1.v +++ b/src/test/test-files/ModuleInstantiator.test.1.v @@ -293,3 +293,59 @@ module golden( assign c = tmp_c; endmodule + + +// --------------------------------------------------------------- +// -- Example of ports with unpacked dimensions and brackets +// --------------------------------------------------------------- +module arrer ( + input clk, + input reset, + input [(2+2)-1:0] a [2:0], + input [3:0] b [(3-1):0] , + input valid, + output [6:0] c + ); + + input clk; + input reset; + input [3:0] a; + // keep this single comment + input [3:0] b; + /* multiline comment should + be kept*/ + input valid; + output [6:0] c; + + reg [6:0] tmp_c; + assign c = tmp_c; + +endmodule + +// --------------------------------------------------------------- +// -- Example of parameters with brackets +// --------------------------------------------------------------- +module azzer #(parameter SIZE = (2*1)+1, + parameter SIZE_TWO = 2*(1+1)) ( + input clk, + input reset, + input [3:0] a, + input [3:0] b, + input valid, + output [6:0] c + ); + + input clk; + input reset; + input [3:0] a; + // keep this single comment + input [3:0] b; + /* multiline comment should + be kept*/ + input valid; + output [6:0] c; + + reg [6:0] tmp_c; + assign c = tmp_c; + +endmodule diff --git a/src/test/test-files/ModuleInstantiator.test.2.v b/src/test/test-files/ModuleInstantiator.test.2.v index ea7abc7..fbdcad3 100644 --- a/src/test/test-files/ModuleInstantiator.test.2.v +++ b/src/test/test-files/ModuleInstantiator.test.2.v @@ -132,6 +132,36 @@ golden u_golden ( .c (c) ); +// --------------------------------------------------------------- +// -- Example of ports with unpacked dimensions and brackets +// --------------------------------------------------------------- + +arrer u_arrer ( + .clk (clk), + .reset (reset), + .a (a), + .b (b), + .valid (valid), + .c (c) +); + + +// --------------------------------------------------------------- +// -- Example of parameters with brackets +// --------------------------------------------------------------- + +azzer #( + .SIZE ((2*1)+1), + .SIZE_TWO (2*(1+1)) +) u_azzer ( + .clk (clk), + .reset (reset), + .a (a), + .b (b), + .valid (valid), + .c (c) +); + // ------------------------------------------------------- // -- End file // ------------------------------------------------------- \ No newline at end of file From 6ae13fea761772782891adc0e0c5f2bda9ec1e3d Mon Sep 17 00:00:00 2001 From: richzwart Date: Fri, 5 Jan 2024 21:37:35 -0500 Subject: [PATCH 2/7] Fixed dropping spaces in comments --- src/providers/ModuleInstantiator.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/providers/ModuleInstantiator.ts b/src/providers/ModuleInstantiator.ts index b4c753d..2ec18ae 100644 --- a/src/providers/ModuleInstantiator.ts +++ b/src/providers/ModuleInstantiator.ts @@ -430,7 +430,10 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized } const output = []; - // Filtering out keys beforehand makes it slightly less time consuming to debug + + const keys = container.split(' '); + // Filtering out keys beforehand makes it slightly less time consuming to debug but also breaks the comment grabber so don't do this: + /* const keys = container.split(' ').filter((el) => { if (isEmptyKey(el)) { return false; @@ -438,6 +441,7 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized return true; } }); + */ // This is a bit funky, but the for loop below actually checks if these varaibles // are undefined so it is important that they are initialized as such From 97ae29416ef1637d9ddc6e095dfc83a52860bca2 Mon Sep 17 00:00:00 2001 From: richzwart Date: Mon, 8 Jan 2024 09:53:11 -0500 Subject: [PATCH 3/7] lint fix --- src/providers/ModuleInstantiator.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/providers/ModuleInstantiator.ts b/src/providers/ModuleInstantiator.ts index 2ec18ae..9640d5e 100644 --- a/src/providers/ModuleInstantiator.ts +++ b/src/providers/ModuleInstantiator.ts @@ -432,16 +432,6 @@ function parseContainer(symbol: string, container: string, moduleIsParameterized const output = []; const keys = container.split(' '); - // Filtering out keys beforehand makes it slightly less time consuming to debug but also breaks the comment grabber so don't do this: - /* - const keys = container.split(' ').filter((el) => { - if (isEmptyKey(el)) { - return false; - } else { - return true; - } - }); - */ // This is a bit funky, but the for loop below actually checks if these varaibles // are undefined so it is important that they are initialized as such From 47140866fe93a058ce3bf1b38652d5c329f96fba Mon Sep 17 00:00:00 2001 From: Joe Crop Date: Mon, 8 Jan 2024 08:10:53 -0800 Subject: [PATCH 4/7] cleaned up tests --- src/test/ModuleInstantiator.test.ts | 6 ++--- src/test/indexer_map.test.ts | 22 +++++++++---------- .../test-files/ModuleInstantiator.test.2.v | 12 +++++----- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/test/ModuleInstantiator.test.ts b/src/test/ModuleInstantiator.test.ts index 3b27910..cde5985 100644 --- a/src/test/ModuleInstantiator.test.ts +++ b/src/test/ModuleInstantiator.test.ts @@ -206,14 +206,14 @@ suite('ModuleInstantiator Tests', () => { let fullRange = null; // Range of the module in the document - fullRange = new vscode.Range(new vscode.Position(301, 6), new vscode.Position(325, 0)); + fullRange = new vscode.Range(new vscode.Position(300, 6), new vscode.Position(324, 0)); let container = document.getText(fullRange).replace(/^\s+|\s+$/g, ''); uri = vscode.Uri.file(path.join(__dirname, testFolderLocation, 'test-files', 'ModuleInstantiator.test.2.v')); document = await vscode.workspace.openTextDocument(uri); - fullRange = new vscode.Range(new vscode.Position(138, 0), new vscode.Position(147, 0)); + fullRange = new vscode.Range(new vscode.Position(137, 0), new vscode.Position(146, 0)); let instance = document.getText(fullRange); @@ -226,7 +226,7 @@ suite('ModuleInstantiator Tests', () => { let fullRange = null; // Range of the module in the document - fullRange = new vscode.Range(new vscode.Position(152, 6), new vscode.Position(164, 0)); + fullRange = new vscode.Range(new vscode.Position(328, 6), new vscode.Position(353, 0)); let container = document.getText(fullRange).replace(/^\s+|\s+$/g, ''); diff --git a/src/test/indexer_map.test.ts b/src/test/indexer_map.test.ts index b4d396c..4a2872b 100644 --- a/src/test/indexer_map.test.ts +++ b/src/test/indexer_map.test.ts @@ -31,10 +31,10 @@ suite('indexer_map Tests', () => { assert.strictEqual(symbols.size, 4); let count = await indexer.addDocumentSymbols(sVDocument, symbols); - assert.strictEqual(count, 8); + assert.strictEqual(count, 10); assert.strictEqual(symbols.size, 5); - assert.strictEqual(symbols.get(uri.fsPath).length, 8); - assert.strictEqual(getSymbolsCount(), 21); + assert.strictEqual(symbols.get(uri.fsPath).length, 10); + assert.strictEqual(getSymbolsCount(), 23); documentSymbols.forEach((symbolName) => { if (!symbolExists(symbolName)) { @@ -47,28 +47,28 @@ suite('indexer_map Tests', () => { assert.strictEqual(count, 0); assert.strictEqual(symbols.size, 5); assert.strictEqual(symbols.get(nonSVUri.fsPath), undefined); - assert.strictEqual(getSymbolsCount(), 21); + assert.strictEqual(getSymbolsCount(), 23); // undefined/null document count = await indexer.addDocumentSymbols(undefined, symbols); assert.strictEqual(count, 0); assert.strictEqual(symbols.size, 5); - assert.strictEqual(getSymbolsCount(), 21); + assert.strictEqual(getSymbolsCount(), 23); count = await indexer.addDocumentSymbols(sVDocument, undefined); assert.strictEqual(count, 0); assert.strictEqual(symbols.size, 5); - assert.strictEqual(getSymbolsCount(), 21); + assert.strictEqual(getSymbolsCount(), 23); count = await indexer.addDocumentSymbols(undefined, undefined); assert.strictEqual(count, 0); assert.strictEqual(symbols.size, 5); - assert.strictEqual(getSymbolsCount(), 21); + assert.strictEqual(getSymbolsCount(), 23); count = await indexer.addDocumentSymbols(null, symbols); assert.strictEqual(count, 0); assert.strictEqual(symbols.size, 5); - assert.strictEqual(getSymbolsCount(), 21); + assert.strictEqual(getSymbolsCount(), 23); }); test('test #2: removeDocumentSymbols', async () => { @@ -80,9 +80,9 @@ suite('indexer_map Tests', () => { assert.strictEqual(symbols.size, 4); let count = await indexer.addDocumentSymbols(sVDocument, symbols); - assert.strictEqual(count, 8); + assert.strictEqual(count, 10); assert.strictEqual(symbols.size, 5); - assert.strictEqual(getSymbolsCount(), 21); + assert.strictEqual(getSymbolsCount(), 23); count = indexer.removeDocumentSymbols(sVDocument.uri.fsPath, symbols); @@ -92,7 +92,7 @@ suite('indexer_map Tests', () => { } }); - assert.strictEqual(count, -8); + assert.strictEqual(count, -10); assert.strictEqual(symbols.size, 4); assert.strictEqual(getSymbolsCount(), 13); diff --git a/src/test/test-files/ModuleInstantiator.test.2.v b/src/test/test-files/ModuleInstantiator.test.2.v index fbdcad3..52c8a0d 100644 --- a/src/test/test-files/ModuleInstantiator.test.2.v +++ b/src/test/test-files/ModuleInstantiator.test.2.v @@ -137,12 +137,12 @@ golden u_golden ( // --------------------------------------------------------------- arrer u_arrer ( - .clk (clk), - .reset (reset), - .a (a), - .b (b), - .valid (valid), - .c (c) + .clk (clk), + .reset (reset), + .a (a), + .b (b), + .valid (valid), + .c (c) ); From 5badfebc2e5703449f71869014b2dd9b5f5176bf Mon Sep 17 00:00:00 2001 From: Joe Crop Date: Mon, 8 Jan 2024 08:17:23 -0800 Subject: [PATCH 5/7] corrected file positions to match other tests --- src/test/ModuleInstantiator.test.ts | 2 +- src/test/test-files/ModuleInstantiator.test.1.v | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/ModuleInstantiator.test.ts b/src/test/ModuleInstantiator.test.ts index cde5985..9511ab8 100644 --- a/src/test/ModuleInstantiator.test.ts +++ b/src/test/ModuleInstantiator.test.ts @@ -233,7 +233,7 @@ suite('ModuleInstantiator Tests', () => { uri = vscode.Uri.file(path.join(__dirname, testFolderLocation, 'test-files', 'ModuleInstantiator.test.2.v')); document = await vscode.workspace.openTextDocument(uri); - fullRange = new vscode.Range(new vscode.Position(152, 0), new vscode.Position(164, 0)); + fullRange = new vscode.Range(new vscode.Position(151, 0), new vscode.Position(163, 0)); let instance = document.getText(fullRange); diff --git a/src/test/test-files/ModuleInstantiator.test.1.v b/src/test/test-files/ModuleInstantiator.test.1.v index 0296711..dbb5969 100644 --- a/src/test/test-files/ModuleInstantiator.test.1.v +++ b/src/test/test-files/ModuleInstantiator.test.1.v @@ -298,6 +298,7 @@ endmodule // --------------------------------------------------------------- // -- Example of ports with unpacked dimensions and brackets // --------------------------------------------------------------- + module arrer ( input clk, input reset, @@ -325,6 +326,7 @@ endmodule // --------------------------------------------------------------- // -- Example of parameters with brackets // --------------------------------------------------------------- + module azzer #(parameter SIZE = (2*1)+1, parameter SIZE_TWO = 2*(1+1)) ( input clk, From cf3288a1f21e232517fd07e5399208a02eecd330 Mon Sep 17 00:00:00 2001 From: richzwart Date: Mon, 8 Jan 2024 22:17:12 -0500 Subject: [PATCH 6/7] Fixed testcase --- src/test/ModuleInstantiator.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ModuleInstantiator.test.ts b/src/test/ModuleInstantiator.test.ts index 9511ab8..d34c977 100644 --- a/src/test/ModuleInstantiator.test.ts +++ b/src/test/ModuleInstantiator.test.ts @@ -226,7 +226,7 @@ suite('ModuleInstantiator Tests', () => { let fullRange = null; // Range of the module in the document - fullRange = new vscode.Range(new vscode.Position(328, 6), new vscode.Position(353, 0)); + fullRange = new vscode.Range(new vscode.Position(327, 6), new vscode.Position(353, 0)); let container = document.getText(fullRange).replace(/^\s+|\s+$/g, ''); From 1271df31ac157afa3dfc1da2f06e9cb50b467b7d Mon Sep 17 00:00:00 2001 From: richzwart Date: Mon, 8 Jan 2024 22:33:39 -0500 Subject: [PATCH 7/7] try again --- src/test/ModuleInstantiator.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ModuleInstantiator.test.ts b/src/test/ModuleInstantiator.test.ts index d34c977..732b273 100644 --- a/src/test/ModuleInstantiator.test.ts +++ b/src/test/ModuleInstantiator.test.ts @@ -226,7 +226,7 @@ suite('ModuleInstantiator Tests', () => { let fullRange = null; // Range of the module in the document - fullRange = new vscode.Range(new vscode.Position(327, 6), new vscode.Position(353, 0)); + fullRange = new vscode.Range(new vscode.Position(329, 6), new vscode.Position(354, 0)); let container = document.getText(fullRange).replace(/^\s+|\s+$/g, '');