Skip to content

token,scanner,parser,checker,cgen: add file_idx to token.Pos#25429

Merged
spytheman merged 3 commits into
vlang:masterfrom
kbkpbot:fix-v-pos-add-file-idx
Oct 2, 2025
Merged

token,scanner,parser,checker,cgen: add file_idx to token.Pos#25429
spytheman merged 3 commits into
vlang:masterfrom
kbkpbot:fix-v-pos-add-file-idx

Conversation

@kbkpbot

@kbkpbot kbkpbot commented Oct 2, 2025

Copy link
Copy Markdown
Contributor

The original token.Pos does not include file name information.
This PR adds a file_idx field to token.Pos, allowing us to identify which source file each token belongs to.

@vlang vlang deleted a comment from huly-for-github Bot Oct 2, 2025
Comment thread vlib/v/token/pos.v Outdated
@spytheman spytheman merged commit d948bf1 into vlang:master Oct 2, 2025
83 checks passed
spytheman added a commit that referenced this pull request Oct 2, 2025
…25429)" (performance degraded, without an immediate clear benefit)

This reverts commit d948bf1.
@spytheman

Copy link
Copy Markdown
Contributor

I've reverted it, because it caused a performance regression, without a clear benefit for correctness or usability:
image

@kbkpbot

kbkpbot commented Oct 3, 2025

Copy link
Copy Markdown
Contributor Author

Occasionally, I encountered issues where error or warning messages pointed to incorrect code locations.

I believe this happens because the error function relies on c.file.path to work properly, but in some cases, this is not a reliable method for obtaining the correct filename.

I will make a note of these cases next time for testing purposes.

@kbkpbot

kbkpbot commented Oct 3, 2025

Copy link
Copy Markdown
Contributor Author

I've reverted it, because it caused a performance regression, without a clear benefit for correctness or usability: image

Can you share the script you use to compare performance?

@spytheman

spytheman commented Oct 3, 2025

Copy link
Copy Markdown
Contributor

Can you share the script you use to compare performance?

The screenshot is from the #fast channel in the V discord server, but you can run something locally, that shows a similar result with:
./v run .github/workflows/compare_pr_to_master.v -prod

(executed on the branch that has the changes)

@spytheman

Copy link
Copy Markdown
Contributor

I suspect, that the slowdown is caused mainly by the increased size of the Pos struct, since it is used a lot in practically every AST node.

If the new file index is encoded in the high 16 bits of the col field, the total size of ast.Pos will be the same, and perhaps the impact will be much lower.
Although I am not sure if just changing it to col u16 and idx i16 may not be enough. It will still allow for 65536 different columns, and 32767 different file indexes.

blackshirt pushed a commit to blackshirt/v that referenced this pull request Oct 4, 2025
…lang#25429)" (performance degraded, without an immediate clear benefit)

This reverts commit d948bf1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants