Skip to content
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

Remove offsets from InstId formatting, trying to name more #4645

Merged
merged 4 commits into from
Dec 6, 2024

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Dec 6, 2024

The offsets were originally added to deal with churn from builtins in the raw semir. In textual semir, we mostly see instruction IDs for imports, and builtins have also settled down more.

On imports, where possible, use the EntityNameId for an import instead of printing an instruction. Next, show the source location if we have a node. Only show the instruction if there's no location.

This also exposes Parse::Tree and TokenizedBuffer, so that we can pass a SemIR::File without the component parts. In particular this allows us to get the TokenizedBuffer for import IRs without substantial structural modifications. We may want to make these optional for serialized SemIR later, but the nodes/tokens contain source location, which we'd need for debug information -- so it's not clear how much we can really make them optional without substantial information loss.

Reduce arguments to just File in a few spots, as a result of the accompanying TokenizedBuffer and Parse::Tree. Also updates style to pass around const File* where the reference is maintained, instead of const File&.

I was considering keeping a direct reference to the tree and tokens on Context, but initially my thought was it wouldn't make much difference. I can re-add those if desired, just as direct caching of the File fields.

@jonmeow jonmeow requested a review from zygoloid December 6, 2024 00:36
@zygoloid zygoloid added this pull request to the merge queue Dec 6, 2024
Merged via the queue into carbon-language:trunk with commit e7a86b0 Dec 6, 2024
8 checks passed
@jonmeow jonmeow deleted the format-inst-offset branch December 6, 2024 21:36
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