Summary
The reorganize_definitions refactor transform assumes each type name has one definition crate-wide. When translation units were compiled with different configurations, the same C struct tag can legitimately have two incompatible declarations, and the transform handles that incoherently: it produces a crate that no longer compiles.
Concrete case (libgit2, PR #1861)
libgit2 compiles its test sources with GIT_DEPRECATE_HARD but its library sources without it (the library implements the deprecated APIs). Under that macro, several public structs change shape by field name/type while staying layout-identical, e.g. git_remote_callbacks.update_tips (function pointer) becomes .reserved_update_tips (void *). The transpiled crate therefore contains two structurally incompatible families of declarations for the same names.
reorganize_definitions correctly refuses to merge the incompatible copies and splits them (git_remote / git_remote_1, cascading through ~24 types that reference each other). The bug is that its per-item merge decisions are not globally coherent:
- Opaque foreign types match any ADT with the same name (
structural_eq_tys_impl, Foreign-vs-Adt arm in context.rs), so TUs that only see a type as an opaque forward declaration get glued to an arbitrary family.
foreign_equiv matches foreign fn declarations to definitions by name only, unifying declarations whose signatures were written against different families.- Within one TU, one item can match family A while a sibling item matches family B, so a kept definition ends up mixing families internally (observed:
git_clone_options_1 with fetch_opts: git_fetch_options_1 but remote_cb: git_remote_create_cb).
Result: E0308/E0605 mismatches at call/cast sites (expected git_remote, found git_remote_1). In CI this surfaced even less legibly: three transforms later, remove_literal_suffixes panicked with "mismatched key trees" (see #1898, which makes the pipeline fail at the transform actually at fault).
What doesn't fix it
Tightening individual comparisons just moves the incoherence around: an experiment making foreign_equiv compare resolved fn signatures (via compatible_fn_sigs) changed the failure from 7 errors to 52. Since the per-TU configuration is what defines a consistent 'universe', a real fix likely needs the matching phase to assign whole TUs (or at least connected components of references) to families consistently, rather than deciding per item.
Reproducing
On #1861 before its -UGIT_DEPRECATE_HARD transpile workaround: transpile libgit2 with --reorganize-definitions --disable-refactoring, then run c2rust refactor rename_unnamed reorganize_definitions --cargo --lib; the crate no longer builds. With the workaround the input crate has a single universe and the transform works, so this is not currently blocking, but any codebase whose TUs see different #ifdef views of a shared header can trigger it.
Related
Summary
The
reorganize_definitionsrefactor transform assumes each type name has one definition crate-wide. When translation units were compiled with different configurations, the same C struct tag can legitimately have two incompatible declarations, and the transform handles that incoherently: it produces a crate that no longer compiles.Concrete case (libgit2, PR #1861)
libgit2 compiles its test sources with
GIT_DEPRECATE_HARDbut its library sources without it (the library implements the deprecated APIs). Under that macro, several public structs change shape by field name/type while staying layout-identical, e.g.git_remote_callbacks.update_tips(function pointer) becomes.reserved_update_tips(void *). The transpiled crate therefore contains two structurally incompatible families of declarations for the same names.reorganize_definitionscorrectly refuses to merge the incompatible copies and splits them (git_remote/git_remote_1, cascading through ~24 types that reference each other). The bug is that its per-item merge decisions are not globally coherent:structural_eq_tys_impl, Foreign-vs-Adt arm incontext.rs), so TUs that only see a type as an opaque forward declaration get glued to an arbitrary family.foreign_equivmatches foreign fn declarations to definitions by name only, unifying declarations whose signatures were written against different families.git_clone_options_1withfetch_opts: git_fetch_options_1butremote_cb: git_remote_create_cb).Result:
E0308/E0605mismatches at call/cast sites (expected git_remote, found git_remote_1). In CI this surfaced even less legibly: three transforms later,remove_literal_suffixespanicked with "mismatched key trees" (see #1898, which makes the pipeline fail at the transform actually at fault).What doesn't fix it
Tightening individual comparisons just moves the incoherence around: an experiment making
foreign_equivcompare resolved fn signatures (viacompatible_fn_sigs) changed the failure from 7 errors to 52. Since the per-TU configuration is what defines a consistent 'universe', a real fix likely needs the matching phase to assign whole TUs (or at least connected components of references) to families consistently, rather than deciding per item.Reproducing
On #1861 before its
-UGIT_DEPRECATE_HARDtranspile workaround: transpile libgit2 with--reorganize-definitions --disable-refactoring, then runc2rust refactor rename_unnamed reorganize_definitions --cargo --lib; the crate no longer builds. With the workaround the input crate has a single universe and the transform works, so this is not currently blocking, but any codebase whose TUs see different#ifdefviews of a shared header can trigger it.Related
libgit2#1861 — libgit2 integration test, including the-UGIT_DEPRECATE_HARDtranspile-time workaround (see conf.yml comment)