Skip to content

Improved error parsing when using verilator #491

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

Merged
merged 2 commits into from
Jul 30, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 50 additions & 14 deletions src/linter/VerilatorLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,42 +107,78 @@
command,
{ cwd: cwd },
(_error: Error, _stdout: string, stderr: string) => {
let diagnostics: vscode.Diagnostic[] = [];
//let diagnostics: vscode.Diagnostic[] = [];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to leave the old code in the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed


// basically DiagnosticsCollection but with ability to append diag lists
let filesDiag = {};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Map instead of Object

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

let error_warning_counter = 0;

Check warning on line 114 in src/linter/VerilatorLinter.ts

View workflow job for this annotation

GitHub Actions / Upload vsix package (macos-latest)

Variable name `error_warning_counter` must match one of the following formats: camelCase, UPPER_CASE

Check warning on line 114 in src/linter/VerilatorLinter.ts

View workflow job for this annotation

GitHub Actions / Upload vsix package (ubuntu-latest)

Variable name `error_warning_counter` must match one of the following formats: camelCase, UPPER_CASE

Check warning on line 114 in src/linter/VerilatorLinter.ts

View workflow job for this annotation

GitHub Actions / Upload vsix package (windows-latest)

Variable name `error_warning_counter` must match one of the following formats: camelCase, UPPER_CASE
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use camelCase for variable name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

stderr.split(/\r?\n/g).forEach((line, _) => {
if (line.search("No such file or directory") >= 0 || line.search("Not a directory") >= 0 || line.search("command not found") >= 0) {
this.logger.error(`Could not execute command: ${command}`);
return;
}

if (!line.startsWith('%') || line.indexOf(docUri) <= 0) {
if (!line.startsWith('%')) {
this.logger.error(line);
return;
}

// for this regex, match sections are:
// 1 - severity (warning/error)
// 3 - error/warning code (EOFNEWLINE, ...),
// 5 - file path
// 9 - line number
// 11 - column number
// 12 - message

let rex = line.match(
/%(\w+)(-[A-Z0-9_]+)?:\s*(\w+:)?(?:[^:]+):\s*(\d+):(?:\s*(\d+):)?\s*(\s*.+)/
/(\w+)(-([A-Z0-9]+))?: ((\S+((\.sv)|(\.v))):(\d+):((\d+):)? )?(.*)/
);

// vscode problems are tied to files, so if there is no file name, no point adding
if (!rex[5]) {return;}

// replacing "\\" and "\" with "/" for consistency
if (isWindows)
{
rex[5] = rex[5].replace(/(\\\\)|(\\)/, "/");
}

// if no errors for this file, new list needs to be created
if (!(rex[5] in Object.keys(filesDiag)))
{
filesDiag[rex[5]] = [];
}


if (rex && rex[0].length > 0) {
let lineNum = Number(rex[4]) - 1;
let colNum = Number(rex[5]) - 1;
let lineNum = Number(rex[9]) - 1;
let colNum = Number(rex[11]) - 1;
// Type of warning is in rex[2]
colNum = isNaN(colNum) ? 0 : colNum; // for older Verilator versions (< 4.030 ~ish)

if (!isNaN(lineNum)) {
diagnostics.push({
filesDiag[rex[5]].push({
severity: this.convertToSeverity(rex[1]),
range: new vscode.Range(lineNum, colNum, lineNum, Number.MAX_VALUE),
message: rex[6],
code: 'verilator',
message: rex[12],
code: rex[3],
source: 'verilator',
});

error_warning_counter++;
}
return;
}
this.logger.warn('[verilator] failed to parse error: ' + line);
});
this.logger.info(`[verilator] ${diagnostics.length} errors/warnings returned`);
this.diagnosticCollection.set(doc.uri, diagnostics);
this.logger.info(`[verilator] ${error_warning_counter} errors/warnings returned`);
Copy link
Owner

@mshr-h mshr-h Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be something like this instead of counting error/warning.

[...filesDiag.values()].reduce((total, arr) => total + arr.length, 0))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After it's done, Verilator prints something like %Error: Exiting due to 3 error(s) and 2 warning(s), which is passed through to the user (well, console). This message is redundant and less informative, thus has been removed entirely.

this.diagnosticCollection.clear()
for (let fileName in filesDiag)
{
let fileURI = vscode.Uri.file(fileName);
// adding diag info for each file
this.diagnosticCollection.set(
fileURI,
filesDiag[fileName]
);
}
}
);
}
Expand Down
Loading