Skip to content

exporter: print src loc for all warnings, including in type encoding#1416

Merged
kkysen merged 6 commits into
masterfrom
kkysen/exporter-print-warning-source-location
Oct 23, 2025
Merged

exporter: print src loc for all warnings, including in type encoding#1416
kkysen merged 6 commits into
masterfrom
kkysen/exporter-print-warning-source-location

Conversation

@kkysen
Copy link
Copy Markdown
Contributor

@kkysen kkysen commented Oct 17, 2025

No description provided.

@kkysen kkysen requested a review from fw-immunant October 17, 2025 10:34
Copy link
Copy Markdown
Contributor

@fw-immunant fw-immunant left a comment

Choose a reason for hiding this comment

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

Code changes look fine, but I don't fully understand the motivation. Is this a pure refactor, or fixing some particular place where our output was useless/incorrect before?

Copy link
Copy Markdown
Contributor Author

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Is this a pure refactor, or fixing some particular place where our output was useless/incorrect before?

I was getting errors, like the "Encountered unsupported BuiltinType kind" one, where there was no source location. clang::FullSourceLoc() was being used, but that's an invalid location. So this threads through the right source location for that and any other errors that occur in the TypeEncoder.

@kkysen kkysen force-pushed the kkysen/exporter-print-warning-source-location branch from 5a94e30 to 6c0107d Compare October 23, 2025 19:38
@kkysen kkysen force-pushed the kkysen/exporter-print-warning-source-location branch from 6c0107d to 68b50af Compare October 23, 2025 19:40
@kkysen kkysen merged commit e529c77 into master Oct 23, 2025
5 checks passed
@kkysen kkysen deleted the kkysen/exporter-print-warning-source-location branch October 23, 2025 21:35
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