Skip to content

[ffigen] Fix record use mapping#3316

Merged
dcharkes merged 1 commit into
mainfrom
ffigen-fix-record-use-mapping
Apr 21, 2026
Merged

[ffigen] Fix record use mapping#3316
dcharkes merged 1 commit into
mainfrom
ffigen-fix-record-use-mapping

Conversation

@dcharkes
Copy link
Copy Markdown
Collaborator

@dcharkes dcharkes commented Apr 17, 2026

Generate the mapping after FFIgen has run, so that we can properly rely on the renamers.

Fixes the code assets sample. (Note this sample isn't tested with dart on the CI because record use is still experimental and that would break on changes.)

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 17, 2026

PR Health

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
code_assets None 1.0.0 1.0.0 1.0.0 ✔️

This check can be disabled by tagging the PR with skip-breaking-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package Leaked API symbol Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

@dcharkes dcharkes force-pushed the ffigen-fix-record-use-mapping branch from e6201b6 to a64ee6d Compare April 17, 2026 12:14
@dcharkes dcharkes requested a review from goderbauer April 17, 2026 13:03
@dcharkes dcharkes marked this pull request as ready for review April 17, 2026 13:07

/// The resolved name of the function as written in the bindings, used for
/// record use mapping.
String? resolvedRecordUseKey;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't like having state that's set during codegen. I think the correct approach is to make funcVarName a proper Symbol, rather than an ad-hoc private name. Then resolvedRecordUseKey can be deleted and recordUseMapping can determine the key itself. This would also let you revert the changes in writer.dart.

Aside from code cleanliness, this is also a better approach because Scope.addPrivate is only supposed to be used for internal names. But the record use map is now exposing that symbol, so it should use the Symbol infra.

@dcharkes dcharkes force-pushed the ffigen-fix-record-use-mapping branch from a64ee6d to 447cdf4 Compare April 20, 2026 07:12
@dcharkes dcharkes force-pushed the ffigen-fix-record-use-mapping branch from 447cdf4 to 3b9f355 Compare April 20, 2026 07:17
@dcharkes dcharkes requested a review from liamappelbe April 20, 2026 07:38
@dcharkes dcharkes merged commit fb88e4a into main Apr 21, 2026
44 checks passed
@dcharkes dcharkes deleted the ffigen-fix-record-use-mapping branch April 21, 2026 06:50
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