refactor: fix nondeterministic ordering for foreign items - #1930
Open
ahomescu wants to merge 2 commits into
Open
Conversation
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 01:40
a977cb9 to
6215ac5
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 01:57
6215ac5 to
52adae1
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 02:07
52adae1 to
6a8430a
Compare
ahomescu
changed the base branch from
ahomescu/fix_reorganize_definitions/move_impls
to
ahomescu/fix_reorganize_definitions/multi_namespace_import_splitting
July 25, 2026 02:11
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 02:11
6a8430a to
1606a2e
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 05:09
1606a2e to
544e434
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 05:33
544e434 to
e433011
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 05:41
e433011 to
52aa47f
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 05:42
52aa47f to
2ce43e0
Compare
ahomescu
changed the base branch from
ahomescu/fix_reorganize_definitions/multi_namespace_import_splitting
to
ahomescu/fix_reorganize_definitions/extern_abi_fallback
July 25, 2026 05:48
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 05:48
2ce43e0 to
ed36074
Compare
ahomescu
changed the base branch from
ahomescu/fix_reorganize_definitions/extern_abi_fallback
to
ahomescu/fix_reorganize_definitions/non_ascii_idents
July 25, 2026 05:52
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 05:52
ed36074 to
0dad64a
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
2 times, most recently
from
July 25, 2026 06:05
ebfa817 to
a24258d
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 06:16
a24258d to
57d0cdc
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 06:26
57d0cdc to
6c83565
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 06:43
6c83565 to
20e73e9
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 25, 2026 06:51
20e73e9 to
a83ee41
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
2 times, most recently
from
July 30, 2026 00:19
ba48462 to
8dafec5
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
July 30, 2026 00:31
8dafec5 to
8df00b2
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
2 times, most recently
from
July 30, 2026 22:56
caa9b42 to
9af9faf
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
2 times, most recently
from
August 4, 2026 02:41
619c11b to
6737ee6
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
2 times, most recently
from
August 5, 2026 00:19
c13616e to
37567a7
Compare
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
August 5, 2026 00:22
37567a7 to
b23392a
Compare
Regression test: `test_reorganize_extern_block_order` (`tests/snapshots/reorganize_extern_block_order.rs`). Two `/usr/include` headers are used so both merge into the same `stdlib` destination — that is the only way to get declarations from two headers into one `into_items` batch, since the normal destination heuristic only matches a header against the module sharing its name. One header declares foreign items in two ABIs (`"C"` and `"sysv64"`) so both failure modes show up at once.
`foreign_items` was a `HashMap<Abi, Vec<_>>` iterated to build the `extern` blocks, so their order varied run to run. Separately, the `BEGIN/END` header comments were assigned while walking the *sorted interleaved* declaration sequence, but foreign items were then regrouped by ABI and hoisted ahead of all items, so the comments described a layout that was never emitted. `into_items` now keys the map on `(header_ident, Abi)` rather than `Abi` alone, and uses an `IndexMap`, so the groups are emitted in the order they are first encountered — which the sort just above derives deterministically from the header line and `src_loc`. Keying on the header too means every `extern` block belongs to exactly one header, which is what lets a single header comment describe it; previously a block could span several headers, and no comment could have been correct for it. The `BEGIN`/`END` comments are then assigned in a second pass over the final `foreign_mods.chain(items)` sequence, so they describe the layout as emitted. Because the blocks are still hoisted ahead of the regular items, a header can now legitimately be entered, left and entered again in one module; the comments say so. Each `extern` block also needs a `NodeId` to hang its comment on, and comments on the foreign items *inside* a block never reach the output (the block is pretty-printed as one new item). `CommandState::next_node_id` only hands out an unused id, so `NodeMap` never learns the node exists and `add_comment` silently drops anything keyed on it — the warning at `command.rs:485` fires. Added `CommandState::register_new_node_id`, which registers the id the same way parsed nodes are registered, and the transform calls it for each block it creates.
ahomescu
force-pushed
the
ahomescu/fix_reorganize_definitions/deterministic_externs
branch
from
August 5, 2026 00:43
b23392a to
e166ab7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack created with GitHub Stacks CLI • Give Feedback 💬