Skip to content

Conversation

fabianschuiki
Copy link
Contributor

The ImportVerilog conversion currently uses a hacky bufferFilePaths table to map from slang::BufferIDs to the corresponding file path. This only works for files that we explicitly add to the source manager. Files discovered automatically by Slang, e.g. through the -y flag, have no entry in that table. As a result, any errors in such files will report their file path as -, which is not very helpful.

This commit fixes this by using whatever Slang's own SourceManager::getFileName returns for a given location. This removes the need for that bufferFilePaths table. As a result, errors in files discovered via -y will now properly show the file name.

While we're at it, also attach the include file stack as a note to any reported diagnostic. This makes it easier to understand how the compiler arrived at an error location.

Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

LGTM! Could you add a test for this with a generic operation format?

The ImportVerilog conversion currently uses a hacky `bufferFilePaths`
table to map from `slang::BufferID`s to the corresponding file path.
This only works for files that we explicitly add to the source manager.
Files discovered automatically by Slang, e.g. through the `-y` flag,
have no entry in that table. As a result, any errors in such files will
report their file path as `-`, which is not very helpful.

This commit fixes this by using whatever Slang's own
`SourceManager::getFileName` returns for a given location. This removes
the need for that `bufferFilePaths` table. As a result, errors in files
discovered via `-y` will now properly show the file name.

While we're at it, also attach the include file stack as a note to any
reported diagnostic. This makes it easier to understand how the compiler
arrived at an error location.
@fabianschuiki fabianschuiki force-pushed the fschuiki/fix-slang-locs branch from e91a2d9 to b1abd20 Compare August 11, 2025 19:29
@fabianschuiki
Copy link
Contributor Author

Great idea! Added a test 👍

@fabianschuiki fabianschuiki merged commit 4fa60bb into main Aug 11, 2025
6 checks passed
@fabianschuiki fabianschuiki deleted the fschuiki/fix-slang-locs branch August 11, 2025 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants