Skip to content

[native_toolchain_c] Don't pass /INCLUDE per symbol with a generated .def on Windows - #3713

Closed
mosuem wants to merge 2 commits into
dart-lang:mainfrom
mosuem:ntc-windows-treeshake
Closed

mosuem wants to merge 2 commits into
dart-lang:mainfrom
mosuem:ntc-windows-treeshake

Conversation

@mosuem

@mosuem mosuem commented Sep 29, 2026

Copy link
Copy Markdown
Member

Description

On Windows, LinkerOptions.treeshake passes an /INCLUDE:<symbol> flag per kept symbol and a generated module-definition (.def) file that exports the same symbols. The EXPORTS in the .def file already make link.exe resolve (pull in from the archive) and keep every symbol, so the /INCLUDE: flags are redundant.

They are also harmful for large APIs: with thousands of symbols (e.g. BoringSSL or ICU4X bindings), the flags exceed the 32,767 character command-line limit of Windows and the link fails. Packages such as package:boring currently work around this by parsing the COFF archive and writing their own .def file with LinkerOptions.manual. With this change, LinkerOptions.treeshake works for them directly.

Changes:

  • Skip the per-symbol /INCLUDE: flags for cl.exe when the .def file is generated (LinkerOptions.treeshake with non-null symbolsToKeep). LinkerOptions.manual is unchanged.
  • Drop the LIBRARY MyDLL placeholder (and the trailing whitespace) from the generated .def file, so the DLL name comes from /OUT instead of a mismatching MyDLL (LNK4070).
  • Add treeshake_many_symbols_test.dart, which keeps 1,500 of 2,000 long-named plain C functions (without __declspec(dllexport)), i.e. ~110K characters of /INCLUDE: flags before this change.

The existing windows_module_definition_test.dart (and its cross variant for ia32/arm64) already covers that the generated .def alone pulls in a non-exported function from the archive; this PR relies on Windows CI for that, as I only ran the tests on Linux locally.

Not included (possible follow-ups): skipping requested symbols that the archive doesn't define (both /INCLUDE: and .def exports fail with LNK2001 for those; package:boring filters them via the COFF symbol table).

Related Issues

None filed.

PR Checklist

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
  • I've run dart tool/ci.dart --all locally and resolved all issues identified. (Ran dart format and dart analyze --fatal-infos on pkgs/native_toolchain_c.)
  • All existing and new tests are passing. I added new tests to check the change I am making. (Linux locally; Windows via CI.)
  • The PR is actually solving the issue.
  • I have updated CHANGELOG.md for the relevant packages.
  • I have updated the pubspec package version if necessary. (Already 0.19.6-wip.)

….def on MSVC

The generated module-definition file already exports, and therefore keeps,
every symbol. The redundant /INCLUDE flags exceed the 32,767 character
Windows command-line limit for thousands of symbols. Also stop naming the
DLL MyDLL in the generated module-definition file.
@codecov

codecov Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.89%. Comparing base (f99449e) to head (2dd9a69).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3713   +/-   ##
=======================================
  Coverage   87.89%   87.89%           
=======================================
  Files         310      310           
  Lines       19771    19772    +1     
=======================================
+ Hits        17377    17378    +1     
  Misses       2394     2394           
Flag Coverage Δ
native_pkgs_macos 86.34% <0.00%> (-0.02%) ⬇️
native_pkgs_ubuntu 71.97% <0.00%> (-0.01%) ⬇️
native_pkgs_windows 75.32% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
code_assets 94.96% <ø> (ø)
data_assets 91.56% <ø> (ø)
ffi 100.00% <ø> (ø)
ffigen 91.46% <ø> (ø)
hooks 86.40% <ø> (ø)
hooks_runner 89.72% <ø> (ø)
jni 66.34% <ø> (ø)
jni_flutter ∅ <ø> (∅)
jni_util 25.00% <ø> (ø)
jnigen 82.85% <ø> (ø)
json_syntax_generator ∅ <ø> (∅)
native_test_helpers 85.71% <ø> (ø)
native_toolchain_c 95.94% <100.00%> (+<0.01%) ⬆️
objective_c 79.53% <ø> (ø)
pub_formats ∅ <ø> (∅)
record_use 81.48% <ø> (ø)
snippet_tool 66.23% <ø> (ø)
swift2objc 92.95% <ø> (ø)
swiftgen 80.80% <ø> (ø)
test_case_selector 97.28% <ø> (ø)
Files with missing lines Coverage Δ
...e_toolchain_c/lib/src/cbuilder/linker_options.dart 94.18% <100.00%> (+0.06%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mosuem

mosuem commented Sep 30, 2026

Copy link
Copy Markdown
Member Author

Superseded by #3718 (moved to an upstream branch so it can be stacked with #3719).

@mosuem mosuem closed this Sep 30, 2026
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.

1 participant